diff --git a/broadcast.go b/broadcast.go index 141c43e36..cea51ed88 100644 --- a/broadcast.go +++ b/broadcast.go @@ -32,10 +32,10 @@ var NopSerializer Serializer = &nopSerializer{} type nopSerializer struct{} -// Marshal A no-op implementation of Serializer Marshall method. +// Marshal is a no-op implementation of Serializer Marshal method. func (*nopSerializer) Marshal(Message) ([]byte, error) { return nil, nil } -// Unmarshal A no-op implementation of Serializer Unmarshal method. +// Unmarshal is a no-op implementation of Serializer Unmarshal method. func (*nopSerializer) Unmarshal([]byte, Message) error { return nil } // broadcaster is an interface for broadcasting messages. diff --git a/field.go b/field.go index a60022735..3208c7c3d 100644 --- a/field.go +++ b/field.go @@ -109,15 +109,6 @@ type Field struct { // Field options. options FieldOptions - // finalOptions is used with a final call to applyOptions. - // The initial call to applyOptions is made with options - // loaded from the meta file on disk (in the case when - // a field is being re-opened). If the field creator calls - // setOptions before calling Open(), then those options - // will be held in finalOptions, and applied instead of - // those from the meta file. - finalOptions *FieldOptions - bsiGroups []*bsiGroup // Shards with data on any node in the cluster, according to this node. @@ -373,7 +364,7 @@ func newField(holder *Holder, path, index, name string, opts FieldOption) (*Fiel schemator: disco.NopSchemator, serializer: NopSerializer, - options: *applyDefaultOptions(&fo), + options: applyDefaultOptions(&fo), remoteAvailableShards: roaring.NewBitmap(), @@ -567,24 +558,12 @@ func (f *Field) Open() error { return errors.Wrap(err, "creating field dir") } - f.holder.Logger.Debugf("load meta file for index/field: %s/%s", f.index, f.name) - if err := f.loadMeta(); err != nil { - return errors.Wrap(err, "loading meta") - } - f.holder.Logger.Debugf("load available shards for index/field: %s/%s", f.index, f.name) - if err := f.loadAvailableShards(); err != nil { return errors.Wrap(err, "loading available shards") } - // If options were provided using setOptions(), then - // use those instead of the options from the meta file. - if f.finalOptions != nil { - f.options = *f.finalOptions - } - - // Apply the field options loaded from meta (or set via setOptions()). + // Apply the field options loaded from etcd (or set via setOptions()). f.holder.Logger.Debugf("apply options for index/field: %s/%s", f.index, f.name) if err := f.applyOptions(f.options); err != nil { return errors.Wrap(err, "applying options") @@ -751,59 +730,6 @@ func (f *Field) openViews() error { return nil } -// loadMeta reads meta data for the field, if any. -func (f *Field) loadMeta() error { - var pb internal.FieldOptions - - // Read data from meta file. - buf, err := ioutil.ReadFile(filepath.Join(f.path, ".meta")) - if os.IsNotExist(err) { - return nil - } else if err != nil { - return errors.Wrap(err, "reading meta") - } else { - if err := proto.Unmarshal(buf, &pb); err != nil { - return errors.Wrap(err, "unmarshaling") - } - } - - // Since pb.Min and pb.Max were changed to pql.Decimal, - // and since they now have a different protobuf field - // number, an existing meta file may have values in the - // old min/max fields which need to be converted to - // pql.Decimal. - // TODO: we can remove the OldMin/OldMax once we're - // confident no one is still using the older version. - var min pql.Decimal - if pb.Min != nil { - min = pql.NewDecimal(pb.Min.Value, pb.Min.Scale) - } else { - min = pql.NewDecimal(pb.OldMin, pb.Scale) - } - var max pql.Decimal - if pb.Max != nil { - max = pql.NewDecimal(pb.Max.Value, pb.Max.Scale) - } else { - max = pql.NewDecimal(pb.OldMax, pb.Scale) - } - - // Copy metadata fields. - f.options.Type = pb.Type - f.options.CacheType = pb.CacheType - f.options.CacheSize = pb.CacheSize - f.options.Min = min - f.options.Max = max - f.options.Base = pb.Base - f.options.Scale = pb.Scale - f.options.BitDepth = pb.BitDepth - f.options.TimeQuantum = TimeQuantum(pb.TimeQuantum) - f.options.Keys = pb.Keys - f.options.NoStandardView = pb.NoStandardView - f.options.ForeignIndex = pb.ForeignIndex - - return nil -} - // saveMeta writes meta data for the field. func (f *Field) saveMeta() error { path := filepath.Join(f.path, ".meta") @@ -832,7 +758,7 @@ func (f *Field) saveMeta() error { // setOptions saves options for final application during Open(). func (f *Field) setOptions(opts *FieldOptions) { - f.finalOptions = applyDefaultOptions(opts) + f.options = applyDefaultOptions(opts) } // applyOptions configures the field based on opt. @@ -1876,13 +1802,16 @@ func newFieldOptions(opts ...FieldOption) (*FieldOptions, error) { // applyDefaultOptions updates FieldOptions with the default // values if o does not contain a valid type. -func applyDefaultOptions(o *FieldOptions) *FieldOptions { +func applyDefaultOptions(o *FieldOptions) FieldOptions { + if o == nil { + o = &FieldOptions{} + } if o.Type == "" { o.Type = DefaultFieldType o.CacheType = DefaultCacheType o.CacheSize = DefaultCacheSize } - return o + return *o } // encode converts o into its internal representation. diff --git a/field_internal_test.go b/field_internal_test.go index 88529d32d..9f2fd8134 100644 --- a/field_internal_test.go +++ b/field_internal_test.go @@ -15,6 +15,7 @@ package pilosa import ( + "context" "fmt" "math" "os" @@ -207,7 +208,8 @@ func NewTestField(t *testing.T, opts FieldOption) *TestField { if err != nil { t.Fatal(err) } - h := NewHolder(path, nil) + + h := NewHolder(path, DefaultHolderConfig()) panicOn(h.Open()) idx, err := h.CreateIndex("i", IndexOptions{}) @@ -247,7 +249,11 @@ func (f *TestField) Reopen() error { f.parent = nil return err } - if err := f.parent.Open(); err != nil { + schema, err := f.parent.Schemator.Schema(context.Background()) + if err != nil { + return err + } + if err := f.parent.OpenWithSchema(schema[f.parent.name]); err != nil { f.parent = nil return err } @@ -546,7 +552,7 @@ func TestField_ApplyOptions(t *testing.T) { } { fld := &Field{} - fld.options = *applyDefaultOptions(&FieldOptions{}) + fld.options = applyDefaultOptions(&FieldOptions{}) if err := fld.applyOptions(tt.opts); err != nil { t.Fatal(err) diff --git a/holder.go b/holder.go index 67c12e887..f813e1b99 100644 --- a/holder.go +++ b/holder.go @@ -234,7 +234,7 @@ func DefaultHolderConfig() *HolderConfig { OpenTransactionStore: OpenInMemTransactionStore, OpenIDAllocator: func(string) (*idAllocator, error) { return &idAllocator{}, nil }, TranslationSyncer: NopTranslationSyncer, - Serializer: NopSerializer, + Serializer: GobSerializer, Schemator: disco.InMemSchemator, CacheFlushInterval: defaultCacheFlushInterval, StatsClient: stats.NopStatsClient, @@ -1276,7 +1276,7 @@ func (h *Holder) newIndex(path, name string) (*Index, error) { index.Stats = h.Stats.WithTags(fmt.Sprintf("index:%s", index.Name())) index.broadcaster = h.broadcaster index.serializer = h.serializer - index.schemator = h.schemator + index.Schemator = h.schemator index.newAttrStore = h.NewAttrStore index.columnAttrs = h.NewAttrStore(filepath.Join(index.path, ".data")) index.OpenTranslateStore = h.OpenTranslateStore diff --git a/index.go b/index.go index 581c42949..f0b74e733 100644 --- a/index.go +++ b/index.go @@ -54,7 +54,7 @@ type Index struct { columnAttrs AttrStore broadcaster broadcaster - schemator disco.Schemator + Schemator disco.Schemator serializer Serializer Stats stats.StatsClient @@ -99,7 +99,7 @@ func NewIndex(holder *Holder, path, name string) (*Index, error) { holder: holder, trackExistence: true, - schemator: disco.InMemSchemator, + Schemator: disco.InMemSchemator, serializer: NopSerializer, translateStores: make(map[int]TranslateStore), @@ -180,6 +180,20 @@ func (i *Index) Open() error { // OpenWithSchema opens the index and uses the provided schema to verify that // the index's fields are expected. func (i *Index) OpenWithSchema(idx *disco.Index) error { + if idx == nil { + return ErrInvalidSchema + } + + // decode the CreateIndexMessage from the schema data in order to + // get its metadata. + cim, err := decodeCreateIndexMessage(i.serializer, idx.Data) + if err != nil { + return errors.Wrap(err, "decoding create index message") + } + i.createdAt = cim.CreatedAt + i.trackExistence = cim.Meta.TrackExistence + i.keys = cim.Meta.Keys + return i.open(idx) } @@ -195,18 +209,6 @@ func (i *Index) open(idx *disco.Index) (err error) { return errors.Wrap(err, "creating directory") } - if idx != nil { - // decode the CreateIndexMessage from the schema data in order to - // get its metadata. - cim, err := decodeCreateIndexMessage(i.serializer, idx.Data) - if err != nil { - return errors.Wrap(err, "decoding create index message") - } - i.createdAt = cim.CreatedAt - i.trackExistence = cim.Meta.TrackExistence - i.keys = cim.Meta.Keys - } - // we don't want to open *all* the views for each shard, since // most are empty when we are doing time quantums. It slows // down startup dramatically. So we ask for the meta data @@ -217,6 +219,9 @@ func (i *Index) open(idx *disco.Index) (err error) { } i.fieldView2shard = fieldView2shard + // Add index to a map in holder. Used by openFields. + i.holder.addIndex(i) + i.holder.Logger.Debugf("open fields for index: %s", i.name) if err := i.openFields(idx); err != nil { return errors.Wrap(err, "opening fields") @@ -299,22 +304,17 @@ fileLoop: var cfm *CreateFieldMessage = &CreateFieldMessage{} var err error - // Only continue with indexes which are present in the provided, + // Only continue with fields which are present in the provided, // non-nil index schema. The reason we have to check for idx != nil - // here is because there are tests which call index.Open on an index - // with a NopSchemator. A better approach might be for those tests - // to use a mock Schemator which returns a schema containing the - // index. For an example, see TestField_SetTimeQuantum which - // re-opens a field and curiously has to re-open that field's index - // because at some point we introduced a pointer from the field back - // to its index (possibly related to transactions?). + // here is because there are tests which call index.Open without + // having a disco.Index available. if idx != nil { fld, ok := idx.Fields[fi.Name()] if !ok { continue } - // Decode the CreateIndexMessage from the schema data in order to + // Decode the CreateFieldMessage from the schema data in order to // get its metadata. cfm, err = decodeCreateFieldMessage(i.holder.serializer, fld.Data) if err != nil { @@ -354,10 +354,6 @@ fileLoop: // the in-memory map of fields maintained by Index. func (i *Index) openField(mu *sync.Mutex, cfm *CreateFieldMessage, file string) (*Field, error) { mu.Lock() - - // goroutine safe - i.holder.addIndex(i) - fld, err := i.newField(i.fieldPath(filepath.Base(file)), filepath.Base(file)) mu.Unlock() if err != nil { @@ -369,6 +365,7 @@ func (i *Index) openField(mu *sync.Mutex, cfm *CreateFieldMessage, file string) fld.holder = i.holder fld.createdAt = cfm.CreatedAt + fld.options = applyDefaultOptions(cfm.Meta) // open the views we have data for. if err := fld.Open(); err != nil { @@ -416,7 +413,6 @@ func (i *Index) openExistenceField() error { // Close closes the index and its fields. func (i *Index) Close() error { - i.mu.Lock() defer i.mu.Unlock() defer func() { @@ -674,7 +670,7 @@ func (i *Index) persistField(ctx context.Context, cfm *CreateFieldMessage) error if b, err := i.serializer.Marshal(cfm); err != nil { return errors.Wrap(err, "marshaling") - } else if err := i.schemator.CreateField(ctx, cfm.Index, cfm.Field, b); err != nil { + } else if err := i.Schemator.CreateField(ctx, cfm.Index, cfm.Field, b); err != nil { return errors.Wrapf(err, "writing field to disco: %s/%s", cfm.Index, cfm.Field) } return nil @@ -705,6 +701,7 @@ func (i *Index) createField(cfm *CreateFieldMessage, broadcast bool) (*Field, er opt = &FieldOptions{} } + // TODO: can we do a general FieldOption validation here instead of just cache type? if cfm.Field == "" { return nil, errors.New("field name required") } else if opt.CacheType != "" && !isValidCacheType(opt.CacheType) { @@ -763,7 +760,7 @@ func (i *Index) newField(path, name string) (*Field, error) { f.idx = i f.Stats = i.Stats f.broadcaster = i.broadcaster - f.schemator = i.schemator + f.schemator = i.Schemator f.serializer = i.serializer f.rowAttrStore = i.newAttrStore(filepath.Join(f.path, ".data")) f.OpenTranslateStore = i.OpenTranslateStore @@ -799,7 +796,7 @@ func (i *Index) DeleteField(name string) error { delete(i.fields, name) // Delete the field from etcd as the system of record. - if err := i.schemator.DeleteField(context.TODO(), i.name, name); err != nil { + if err := i.Schemator.DeleteField(context.TODO(), i.name, name); err != nil { return errors.Wrapf(err, "deleting field from etcd: %s/%s", i.name, name) } diff --git a/pilosa.go b/pilosa.go index f318a6eb4..370144c9d 100644 --- a/pilosa.go +++ b/pilosa.go @@ -34,6 +34,8 @@ var ( ErrIndexExists = disco.ErrIndexExists ErrIndexNotFound = errors.New("index not found") + ErrInvalidSchema = errors.New("invalid schema") + ErrForeignIndexNotFound = errors.New("foreign index not found") // ErrFieldRequired is returned when no field is specified. diff --git a/test/index.go b/test/index.go index 376ee6d65..d885d05da 100644 --- a/test/index.go +++ b/test/index.go @@ -15,6 +15,7 @@ package test import ( + "context" "testing" "github.com/pilosa/pilosa/v2" @@ -32,7 +33,7 @@ func newIndex(tb testing.TB) *Index { if err != nil { panic(err) } - h := pilosa.NewHolder(path, nil) + h := pilosa.NewHolder(path, pilosa.DefaultHolderConfig()) testhook.Cleanup(tb, func() { h.Close() }) @@ -59,7 +60,11 @@ func (i *Index) Reopen() error { if err := i.Index.Close(); err != nil { return err } - return i.Index.Open() + schema, err := i.Schemator.Schema(context.Background()) + if err != nil { + return err + } + return i.OpenWithSchema(schema[i.Name()]) } // CreateField creates a field with the given options.