From 27739991908d298a4bb138beb824b91ff6df0d16 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Mon, 8 Feb 2021 12:12:34 -0600 Subject: [PATCH] Simplify response structs --- api.go | 6 +++--- cluster.go | 5 ----- executor.go | 2 -- field.go | 24 +++++------------------- holder.go | 35 ----------------------------------- http/handler.go | 2 +- index.go | 15 --------------- server/handler_test.go | 38 +++++++++++++++++++------------------- 8 files changed, 28 insertions(+), 99 deletions(-) diff --git a/api.go b/api.go index 6fa885e01..ff5b7c6b4 100644 --- a/api.go +++ b/api.go @@ -997,10 +997,10 @@ func (api *API) Schema(ctx context.Context) []*IndexInfo { // SchemaDetails returns information about each index in Pilosa including which // fields they contain, and additional field information such as cardinality -func (api *API) SchemaDetails(ctx context.Context) []*IndexDetails { +func (api *API) SchemaDetails(ctx context.Context) []*IndexInfo { span, _ := tracing.StartSpanFromContext(ctx, "API.Schema") defer span.Finish() - schema := api.holder.SchemaDetails() + schema := api.holder.Schema(false) for _, index := range schema { for _, field := range index.Fields { q := fmt.Sprintf("Count(Distinct(field=%s))", field.Name) @@ -1014,7 +1014,7 @@ func (api *API) SchemaDetails(ctx context.Context) []*IndexDetails { continue } if card, ok := resp.Results[0].(uint64); ok { - field.Cardinality = card + field.Cardinality = &card } } } diff --git a/cluster.go b/cluster.go index 45b93ef67..82e2e67a1 100644 --- a/cluster.go +++ b/cluster.go @@ -3134,11 +3134,6 @@ type Schema struct { Indexes []*IndexInfo `json:"indexes"` } -// SchemaDetails contains information about indexes and their configuration. -type SchemaDetails struct { - Indexes []*IndexDetails `json:"indexes"` -} - func encodeTopology(topology *Topology) *internal.Topology { if topology == nil { return nil diff --git a/executor.go b/executor.go index bb17f6261..34fc5c87a 100644 --- a/executor.go +++ b/executor.go @@ -755,7 +755,6 @@ func (e *executor) executeCall(ctx context.Context, qcx *Qcx, index string, c *p case "Distinct": statFn() res, err := e.executeDistinct(ctx, qcx, index, c, shards, opt) - // TODO this can produce an ugly list of 256 shards return res, errors.Wrapf(err, "executeDistinct %v", shardSlice(shards)) case "Store": statFn() @@ -764,7 +763,6 @@ func (e *executor) executeCall(ctx context.Context, qcx *Qcx, index string, c *p case "Count": statFn() res, err := e.executeCount(ctx, qcx, index, c, shards, opt) - // TODO this can produce an ugly list of 256 shards return res, errors.Wrapf(err, "executeCount %v", shardSlice(shards)) case "Set": statFn() diff --git a/field.go b/field.go index f4ab3982b..e3000ca61 100644 --- a/field.go +++ b/field.go @@ -1867,10 +1867,11 @@ func (p fieldSlice) Less(i, j int) bool { return p[i].Name() < p[j].Name() } // FieldInfo represents schema information for a field. type FieldInfo struct { - Name string `json:"name"` - CreatedAt int64 `json:"createdAt,omitempty"` - Options FieldOptions `json:"options"` - Views []*ViewInfo `json:"views,omitempty"` + Name string `json:"name"` + CreatedAt int64 `json:"createdAt,omitempty"` + Options FieldOptions `json:"options"` + Cardinality *uint64 `json:"cardinality,omitempty"` + Views []*ViewInfo `json:"views,omitempty"` } type fieldInfoSlice []*FieldInfo @@ -1879,21 +1880,6 @@ func (p fieldInfoSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } func (p fieldInfoSlice) Len() int { return len(p) } func (p fieldInfoSlice) Less(i, j int) bool { return p[i].Name < p[j].Name } -// FieldDetails represents detailed schema information for a field. -type FieldDetails struct { - Name string `json:"name"` - CreatedAt int64 `json:"createdAt,omitempty"` - Options FieldOptions `json:"options"` - Cardinality uint64 `json:"cardinality"` - Views []*ViewInfo `json:"views,omitempty"` -} - -type fieldDetailsSlice []*FieldDetails - -func (p fieldDetailsSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } -func (p fieldDetailsSlice) Len() int { return len(p) } -func (p fieldDetailsSlice) Less(i, j int) bool { return p[i].Name < p[j].Name } - // FieldOptions represents options to set when initializing a field. type FieldOptions struct { Base int64 `json:"base,omitempty"` diff --git a/holder.go b/holder.go index 1bbb33984..46bacfa93 100644 --- a/holder.go +++ b/holder.go @@ -882,41 +882,6 @@ func (h *Holder) Schema(includeHiddenAndViews bool) []*IndexInfo { return a } -// SchemaDetails returns schema information for all non-hidden indexes and fields, -// including additional per-field details such as cardinality, actual range of integer data, etc. -// This function duplicates the logic of Holder.Schema because the FieldDetails struct -// includes a struct-field for cardinality, with default value 0, so the behavior of omitempty -// is incompatible between the /schema and /schema/details HTTP endpoints. A value of 0 for -// cardinality is meaningful, so it should be included when accurate, and not accidentally -// reported as 0 when the struct-field has not been populated. -func (h *Holder) SchemaDetails() []*IndexDetails { - var a []*IndexDetails - for _, index := range h.Indexes() { - di := &IndexDetails{ - Name: index.Name(), - CreatedAt: index.CreatedAt(), - Options: index.Options(), - ShardWidth: ShardWidth, - Fields: make([]*FieldDetails, 0, len(index.Fields())), - } - for _, field := range index.Fields() { - if strings.HasPrefix(field.name, "_") { - continue - } - fi := &FieldDetails{ - Name: field.Name(), - CreatedAt: field.CreatedAt(), - Options: field.Options(), - } - di.Fields = append(di.Fields, fi) - } - sort.Sort(fieldDetailsSlice(di.Fields)) - a = append(a, di) - } - sort.Sort(indexDetailsSlice(a)) - return a -} - // applySchema applies an internal Schema to Holder. func (h *Holder) applySchema(schema *Schema) error { // Create indexes that don't exist. diff --git a/http/handler.go b/http/handler.go index 2d98a5ec4..14ef35fe1 100644 --- a/http/handler.go +++ b/http/handler.go @@ -682,7 +682,7 @@ func (h *Handler) handleGetSchemaDetails(w http.ResponseWriter, r *http.Request) w.Header().Set("Content-Type", "application/json") schema := h.api.SchemaDetails(r.Context()) - if err := json.NewEncoder(w).Encode(pilosa.SchemaDetails{Indexes: schema}); err != nil { + if err := json.NewEncoder(w).Encode(pilosa.Schema{Indexes: schema}); err != nil { h.logger.Printf("write schema response error: %s", err) } } diff --git a/index.go b/index.go index a290e2d3d..6129289f5 100644 --- a/index.go +++ b/index.go @@ -668,21 +668,6 @@ func (p indexInfoSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } func (p indexInfoSlice) Len() int { return len(p) } func (p indexInfoSlice) Less(i, j int) bool { return p[i].Name < p[j].Name } -// IndexDetails represents detailed schema information for an index. -type IndexDetails struct { - Name string `json:"name"` - CreatedAt int64 `json:"createdAt,omitempty"` - Options IndexOptions `json:"options"` - Fields []*FieldDetails `json:"fields"` - ShardWidth uint64 `json:"shardWidth"` -} - -type indexDetailsSlice []*IndexDetails - -func (p indexDetailsSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } -func (p indexDetailsSlice) Len() int { return len(p) } -func (p indexDetailsSlice) Less(i, j int) bool { return p[i].Name < p[j].Name } - // IndexOptions represents options to set when initializing an index. type IndexOptions struct { Keys bool `json:"keys"` diff --git a/server/handler_test.go b/server/handler_test.go index 8c0c83530..035932699 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -224,6 +224,20 @@ func TestHandler_Endpoints(t *testing.T) { t.Fatal(err) } + t.Run("Schema", func(t *testing.T) { + w := httptest.NewRecorder() + h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/schema", nil)) + if w.Code != gohttp.StatusOK { + t.Fatalf("unexpected status code: %d", w.Code) + } + + body := strings.TrimSpace(w.Body.String()) + target := fmt.Sprintf(`{"indexes":[{"name":"i0","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false}},{"name":"f1","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false}}],"shardWidth":%[1]d},{"name":"i1","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false}}],"shardWidth":%[1]d}]}`, pilosa.ShardWidth) + if body != target { + t.Fatalf("\n%s\n!=\n%s", target, body) + } + }) + // i2 is for SchemaDetails i2 := hldr.MustCreateIndexIfNotExists("i2", pilosa.IndexOptions{}) tx2, err := holder.BeginTx(true, i2.Index, shard) @@ -275,20 +289,6 @@ func TestHandler_Endpoints(t *testing.T) { t.Fatal(err) } - t.Run("Schema", func(t *testing.T) { - w := httptest.NewRecorder() - h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/schema", nil)) - if w.Code != gohttp.StatusOK { - t.Fatalf("unexpected status code: %d", w.Code) - } - - body := strings.TrimSpace(w.Body.String()) - target := fmt.Sprintf(`{"indexes":[{"name":"i0","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false}},{"name":"f1","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false}}],"shardWidth":%d},{"name":"i1","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false}}],"shardWidth":%[1]d}]}`, pilosa.ShardWidth) - if body != target { - t.Fatalf("%s != %s", target, body) - } - }) - t.Run("SchemaDetails", func(t *testing.T) { w := httptest.NewRecorder() h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/schema/details", nil)) @@ -297,7 +297,7 @@ func TestHandler_Endpoints(t *testing.T) { } body := strings.TrimSpace(w.Body.String()) - target := fmt.Sprintf(`{"indexes":[{"name":"i0","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":0},{"name":"f1","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":1}],"shardWidth":1048576},{"name":"i1","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":1}],"shardWidth":1048576},{"name":"i2","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":1000,"keys":false},"cardinality":1},{"name":"f1","options":{"type":"int","base":0,"bitDepth":3,"min":-100,"max":100,"keys":false,"foreignIndex":""},"cardinality":4},{"name":"f2","options":{"type":"decimal","base":0,"scale":1,"bitDepth":7,"min":-10,"max":10,"keys":false},"cardinality":5},{"name":"f3","options":{"type":"time","timeQuantum":"YMDH","keys":false,"noStandardView":false},"cardinality":1},{"name":"f4","options":{"type":"mutex","cacheType":"ranked","cacheSize":5000,"keys":false},"cardinality":1},{"name":"f5","options":{"type":"bool"},"cardinality":1}],"shardWidth":%[1]d}]}`, pilosa.ShardWidth) + target := fmt.Sprintf(`{"indexes":[{"name":"i0","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":0},{"name":"f1","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":1}],"shardWidth":%[1]d},{"name":"i1","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":1}],"shardWidth":%[1]d},{"name":"i2","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":1000,"keys":false},"cardinality":1},{"name":"f1","options":{"type":"int","base":0,"bitDepth":3,"min":-100,"max":100,"keys":false,"foreignIndex":""},"cardinality":4},{"name":"f2","options":{"type":"decimal","base":0,"scale":1,"bitDepth":7,"min":-10,"max":10,"keys":false},"cardinality":5},{"name":"f3","options":{"type":"time","timeQuantum":"YMDH","keys":false,"noStandardView":false},"cardinality":1},{"name":"f4","options":{"type":"mutex","cacheType":"ranked","cacheSize":5000,"keys":false},"cardinality":1},{"name":"f5","options":{"type":"bool"},"cardinality":1}],"shardWidth":%[1]d}]}`, pilosa.ShardWidth) if body != target { t.Fatalf("%s\n!=\n%s", target, body) } @@ -476,13 +476,13 @@ func TestHandler_Endpoints(t *testing.T) { for _, nodeUsage := range nodeUsages { numIndexes := len(nodeUsage.Disk.IndexUsage) - if nodeUsage.Disk.TotalUse < 75000 || nodeUsage.Disk.TotalUse > 300000 { + if nodeUsage.Disk.TotalUse < 75000 || nodeUsage.Disk.TotalUse > 500000 { // Usage measurements are not consistent between machines, or // over time, as features and implementations change, so checking // for a range of sizes may be most useful way to test the details of this. - t.Fatalf("expected 75k < total < 300k, got %d", nodeUsage.Disk.TotalUse) + t.Fatalf("expected 75k < total < 500k, got %d", nodeUsage.Disk.TotalUse) } - if numIndexes != 2 { + if numIndexes != 3 { t.Fatalf("wrong length index usage list: expected %d, got %d", 2, numIndexes) } numFields := len(nodeUsage.Disk.IndexUsage["i1"].Fields) @@ -561,7 +561,7 @@ func TestHandler_Endpoints(t *testing.T) { h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/internal/shards/max", nil)) if w.Code != gohttp.StatusOK { t.Fatalf("unexpected status code: %d", w.Code) - } else if body := w.Body.String(); body != `{"standard":{"i0":3,"i1":0}}`+"\n" { + } else if body := w.Body.String(); body != `{"standard":{"i0":3,"i1":0,"i2":0}}`+"\n" { t.Fatalf("unexpected body: %s", body) } })