Merge pull request #1956 from molecula/fb-1127

remove /schema/details cardinality calculation, and schemaDetailsOn config option
This commit is contained in:
reesporte 2022-03-02 14:37:53 -06:00 committed by GitHub
commit 2f6aed74e6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
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")