remove cardinality calculation from schema/details

this is related to work for [fb-1127](https://molecula.atlassian.net/browse/FB-1127)

cardinality reporting has caused no shortage of issues such that we recommend
disabling them almost everywhere.

this commit removes the cardinality calculation for right now, as well as the option
to enable/disable schema details.
This commit is contained in:
reesporte 2022-03-01 10:11:17 -06:00
parent 239477c526
commit 17203e3441
8 changed files with 10 additions and 118 deletions

42
api.go
View file

@ -50,8 +50,6 @@ type API struct {
importWorkerPoolSize int
importWork chan importJob
schemaDetailsOn bool
Serializer Serializer
}
@ -72,14 +70,6 @@ func OptAPIServer(s *Server) apiOption {
}
}
// Used to configure API option: schemaDetailsOn
func OptAPISchemaDetailsOn(isOn bool) apiOption {
return func(a *API) error {
a.schemaDetailsOn = isOn
return nil
}
}
func OptAPIImportWorkerPoolSize(size int) apiOption {
return func(a *API) error {
a.importWorkerPoolSize = size
@ -1021,38 +1011,6 @@ func (api *API) Schema(ctx context.Context, withViews bool) ([]*IndexInfo, error
return api.holder.limitedSchema()
}
// SchemaDetails returns information about each index in Pilosa including which
// fields they contain. Additional field information such as cardinality unless
// turned off via the schemaDetailsOn cli option.
func (api *API) SchemaDetails(ctx context.Context) ([]*IndexInfo, error) {
span, _ := tracing.StartSpanFromContext(ctx, "API.Schema")
defer span.Finish()
schema, err := api.holder.Schema()
if err != nil {
return nil, errors.Wrap(err, "getting schema")
}
if !api.schemaDetailsOn {
return schema, nil
}
for _, index := range schema {
for _, field := range index.Fields {
q := fmt.Sprintf("Count(Distinct(field=%s))", field.Name)
req := QueryRequest{Index: index.Name, Query: q}
resp, err := api.query(ctx, &req)
if err != nil {
return schema, errors.Wrapf(err, "querying cardinality (%s/%s)", index.Name, field.Name)
}
if len(resp.Results) == 0 {
continue
}
if card, ok := resp.Results[0].(uint64); ok {
field.Cardinality = &card
}
}
}
return schema, nil
}
// ApplySchema takes the given schema and applies it across the
// cluster (if remote is false), or just to this node (if remote is
// true). This is designed for the use case of replicating a schema

View file

@ -956,29 +956,6 @@ func TestAPI_IDAlloc(t *testing.T) {
})
}
func TestAPI_SchemaDetailsOff(t *testing.T) {
cluster := test.MustRunCluster(t, 2)
defer cluster.Close()
cmd := cluster.GetNode(0)
err := cmd.API.SetAPIOptions(pilosa.OptAPISchemaDetailsOn(false))
if err != nil {
t.Fatalf("could not toggle schema details to off: %v", err)
}
schema, err := cmd.API.SchemaDetails(context.Background())
if err != nil {
t.Fatalf("getting schema: %v", err)
}
for _, i := range schema {
for _, f := range i.Fields {
if f.Cardinality != nil {
t.Fatalf("expected nil cardinality, got: %v", *f.Cardinality)
}
}
}
}
type mutexCheckIndex struct {
index *pilosa.Index
indexName string

View file

@ -93,9 +93,6 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) {
// Future flags.
flags.BoolVar(&srv.Config.Future.Rename, "future.rename", false, "Present application name as FeatureBase. Defaults to false, will default to true in an upcoming release.")
// Toggle /schema/details endpoint.
flags.BoolVar(&srv.Config.SchemaDetailsOn, "schema-details-on", true, "Disable /schema/details endpoint")
// OAuth2.0 identity provider configuration
flags.BoolVar(&srv.Config.Auth.Enable, "auth.enable", false, "Enable AuthN/AuthZ of featurebase, disabled by default.")
flags.StringVar(&srv.Config.Auth.ClientId, "auth.client-id", srv.Config.Auth.ClientId, "Identity Provider's Application/Client ID.")

View file

@ -926,7 +926,12 @@ func (h *Handler) handleGetSchema(w http.ResponseWriter, r *http.Request) {
}
}
// handleGetSchema handles GET /schema/details requests.
// handleGetSchema handles GET /schema/details requests. This is essentially the
// same thing as a GET /schema request, except WithViews is turned on by default.
// Previously, /schema/details returned the cardinality of each field, but this was
// removed for performance reasons. If, at some point in the future, there is a more
// performant way to get the cardinality of a field, that information would be
// included here.
func (h *Handler) handleGetSchemaDetails(w http.ResponseWriter, r *http.Request) {
if !validHeaderAcceptJSON(r.Header) {
http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable)
@ -934,7 +939,7 @@ func (h *Handler) handleGetSchemaDetails(w http.ResponseWriter, r *http.Request)
}
w.Header().Set("Content-Type", "application/json")
schema, err := h.api.SchemaDetails(r.Context())
schema, err := h.api.Schema(r.Context(), true)
if err != nil {
h.logger.Printf("error getting detailed schema: %s", err)
return

View file

@ -144,7 +144,6 @@ export const MoleculaTable: FC<MoleculaTableProps> = ({
</span>
</TableCell>
<TableCell className={css.tableHeader}>Type</TableCell>
<TableCell className={css.tableHeader}>Cardinality</TableCell>
<TableCell className={css.tableHeader}>Options</TableCell>
</TableRow>
</TableHead>
@ -171,9 +170,6 @@ export const MoleculaTable: FC<MoleculaTableProps> = ({
{type} {showKeys ? (keys ? '(keys)' : '(ID)') : null}
</code>
</TableCell>
<TableCell className={css.tableCell}>
{cardinality ? cardinality.toLocaleString() : '-'}
</TableCell>
<TableCell className={css.tableCell}>
<div className={css.optionsTable}>
{map(rest, (value, key) => {

View file

@ -222,9 +222,6 @@ type Config struct {
Rename bool `toml:"rename"`
} `toml:"future"`
// Toggles /schema/details endpoint. If off, it returns empty.
SchemaDetailsOn bool `toml:"schema-details-on"`
Auth Auth
}
@ -390,9 +387,6 @@ func NewConfig() *Config {
// Future flags.
c.Future.Rename = false
// Schema Details Toggle
c.SchemaDetailsOn = true
return c
}

View file

@ -302,8 +302,7 @@ func TestHandler_Endpoints(t *testing.T) {
}
var bodySchema pilosa.Schema
if err := json.Unmarshal(w.Body.Bytes(),
&bodySchema); err != nil {
if err := json.Unmarshal(w.Body.Bytes(), &bodySchema); err != nil {
t.Fatalf("unexpected unmarshalling error: %v", err)
}
// DO NOT COMPARE `CreatedAt` - reset to 0
@ -316,9 +315,8 @@ func TestHandler_Endpoints(t *testing.T) {
//
var targetSchema pilosa.Schema
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,"views":[{"name":"standard"}]}],"shardWidth":%[1]d},{"name":"i1","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":1,"views":[{"name":"standard"}]}],"shardWidth":%[1]d},{"name":"i2","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":1000,"keys":false},"cardinality":1,"views":[{"name":"standard"}]},{"name":"f1","options":{"type":"int","base":0,"bitDepth":0,"min":-100,"max":100,"keys":false,"foreignIndex":""},"cardinality":4,"views":[{"name":"bsig_f1"}]},{"name":"f2","options":{"type":"decimal","base":0,"scale":1,"bitDepth":0,"min":-10,"max":10,"keys":false},"cardinality":5,"views":[{"name":"bsig_f2"}]},{"name":"f3","options":{"type":"time","timeQuantum":"YMDH","keys":false,"noStandardView":false},"cardinality":1,"views":[{"name":"standard"}]},{"name":"f4","options":{"type":"mutex","cacheType":"ranked","cacheSize":5000,"keys":false},"cardinality":1,"views":[{"name":"standard"}]},{"name":"f5","options":{"type":"bool"},"cardinality":1,"views":[{"name":"standard"}]}],"shardWidth":%[1]d}]}`, pilosa.ShardWidth)
if err := json.Unmarshal([]byte(target),
&targetSchema); err != nil {
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},"views":[{"name":"standard"}]}],"shardWidth":%[1]d},{"name":"i1","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"views":[{"name":"standard"}]}],"shardWidth":%[1]d},{"name":"i2","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":1000,"keys":false},"views":[{"name":"standard"}]},{"name":"f1","options":{"type":"int","base":0,"bitDepth":0,"min":-100,"max":100,"keys":false,"foreignIndex":""},"views":[{"name":"bsig_f1"}]},{"name":"f2","options":{"type":"decimal","base":0,"scale":1,"bitDepth":0,"min":-10,"max":10,"keys":false},"views":[{"name":"bsig_f2"}]},{"name":"f3","options":{"type":"time","timeQuantum":"YMDH","keys":false,"noStandardView":false},"views":[{"name":"standard"}]},{"name":"f4","options":{"type":"mutex","cacheType":"ranked","cacheSize":5000,"keys":false},"views":[{"name":"standard"}]},{"name":"f5","options":{"type":"bool"},"views":[{"name":"standard"}]}],"shardWidth":%[1]d}]}`, pilosa.ShardWidth)
if err := json.Unmarshal([]byte(target), &targetSchema); err != nil {
t.Fatalf("unexpected unmarshalling error: %v", err)
}
@ -327,38 +325,6 @@ func TestHandler_Endpoints(t *testing.T) {
}
})
t.Run("SchemaDetailsOff", func(t *testing.T) {
err := cmd.API.SetAPIOptions(pilosa.OptAPISchemaDetailsOn(false))
if err != nil {
t.Fatalf("setting schema details option")
}
w := httptest.NewRecorder()
h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/schema/details", nil))
if w.Code != gohttp.StatusOK {
t.Fatalf("unexpected status code: %d", w.Code)
}
var bodySchema pilosa.Schema
if err := json.Unmarshal(w.Body.Bytes(),
&bodySchema); err != nil {
t.Fatalf("unexpected unmarshalling error: %v", err)
}
for _, i := range bodySchema.Indexes {
for _, f := range i.Fields {
if f.Cardinality != nil {
t.Fatalf("expected nil cardinality, got: %v", *f.Cardinality)
}
}
}
err = cmd.API.SetAPIOptions(pilosa.OptAPISchemaDetailsOn(true))
if err != nil {
t.Fatalf("could not toggle schema details to on: %v", err)
}
})
t.Run("Import", func(t *testing.T) {
indexInfo, err := cmd.API.Schema(context.Background(), false)
if err != nil {

View file

@ -509,7 +509,6 @@ func (m *Command) SetupServer() error {
m.API, err = pilosa.NewAPI(
pilosa.OptAPIServer(m.Server),
pilosa.OptAPIImportWorkerPoolSize(m.Config.ImportWorkerPoolSize),
pilosa.OptAPISchemaDetailsOn(m.Config.SchemaDetailsOn),
)
if err != nil {
return errors.Wrap(err, "new api")