diff --git a/disco/disco.go b/disco/disco.go index a4029c97d..117e1f6e3 100644 --- a/disco/disco.go +++ b/disco/disco.go @@ -104,7 +104,7 @@ type Index struct { // for each of its views. type Field struct { Data []byte - Views map[string][]byte + Views map[string]struct{} } type Schemator interface { @@ -115,8 +115,8 @@ type Schemator interface { Field(ctx context.Context, index, field string) ([]byte, error) CreateField(ctx context.Context, index, field string, val []byte) error DeleteField(ctx context.Context, index, field string) error - View(ctx context.Context, index, field, view string) ([]byte, error) - CreateView(ctx context.Context, index, field, view string, val []byte) error + View(ctx context.Context, index, field, view string) (bool, error) + CreateView(ctx context.Context, index, field, view string) error DeleteView(ctx context.Context, index, field, view string) error } @@ -284,12 +284,12 @@ func (*nopSchemator) CreateField(ctx context.Context, index, field string, val [ func (*nopSchemator) DeleteField(ctx context.Context, index, field string) error { return nil } // View is a no-op implementation of the Schemator View method. -func (*nopSchemator) View(ctx context.Context, index, field, view string) ([]byte, error) { - return nil, nil +func (*nopSchemator) View(ctx context.Context, index, field, view string) (bool, error) { + return false, nil } // CreateView is a no-op implementation of the Schemator CreateView method. -func (*nopSchemator) CreateView(ctx context.Context, index, field, view string, val []byte) error { +func (*nopSchemator) CreateView(ctx context.Context, index, field, view string) error { return nil } @@ -383,7 +383,7 @@ func (s *inMemSchemator) CreateField(ctx context.Context, index, field string, v } idx.Fields[field] = &Field{ Data: val, - Views: make(map[string][]byte), + Views: make(map[string]struct{}), } return nil } @@ -401,26 +401,23 @@ func (s *inMemSchemator) DeleteField(ctx context.Context, index, field string) e } // View is an in-memory implementation of the Schemator View method. -func (s *inMemSchemator) View(ctx context.Context, index, field, view string) ([]byte, error) { +func (s *inMemSchemator) View(ctx context.Context, index, field, view string) (bool, error) { s.mu.RLock() defer s.mu.RUnlock() idx, ok := s.schema[index] if !ok { - return nil, ErrIndexDoesNotExist + return false, ErrIndexDoesNotExist } fld, ok := idx.Fields[field] if !ok { - return nil, ErrFieldDoesNotExist + return false, ErrFieldDoesNotExist } - data, ok := fld.Views[view] - if !ok { - return nil, ErrViewDoesNotExist - } - return data, nil + _, ok = fld.Views[view] + return ok, nil } // CreateView is an in-memory implementation of the Schemator CreateView method. -func (s *inMemSchemator) CreateView(ctx context.Context, index, field, view string, val []byte) error { +func (s *inMemSchemator) CreateView(ctx context.Context, index, field, view string) error { s.mu.Lock() defer s.mu.Unlock() idx, ok := s.schema[index] @@ -433,7 +430,7 @@ func (s *inMemSchemator) CreateView(ctx context.Context, index, field, view stri } // The current logic in pilosa doesn't allow us to return ErrViewExists // here, so for now we just update the value if the view already exists. - fld.Views[view] = val + fld.Views[view] = struct{}{} return nil } diff --git a/etcd/embed.go b/etcd/embed.go index dd1062285..2b2b6dc35 100644 --- a/etcd/embed.go +++ b/etcd/embed.go @@ -516,7 +516,7 @@ func (e *Etcd) Schema(ctx context.Context) (disco.Schema, error) { if _, ok := flds[field]; !ok { flds[field] = &disco.Field{ Data: vals[i], - Views: make(map[string][]byte), + Views: make(map[string]struct{}), } continue } @@ -525,7 +525,7 @@ func (e *Etcd) Schema(ctx context.Context) (disco.Schema, error) { // token[3]: view if len(tokens) > 3 { view := tokens[3] - views[view] = vals[i] + views[view] = struct{}{} } } } @@ -672,15 +672,15 @@ func (e *Etcd) DeleteField(ctx context.Context, indexname string, name string) e return errors.Wrap(err, "DeleteField") } -func (e *Etcd) View(ctx context.Context, indexName, fieldName, name string) ([]byte, error) { +func (e *Etcd) View(ctx context.Context, indexName, fieldName, name string) (bool, error) { key := schemaPrefix + indexName + "/" + fieldName + "/" + name - return e.getKeyBytes(ctx, key) + return e.keyExists(ctx, key) } // CreateView differs from CreateIndex and CreateField in that it does not // return an error if the view already exists. If this logic needs to be // changed, we likely need to return disco.ErrViewExists. -func (e *Etcd) CreateView(ctx context.Context, indexName, fieldName, name string, val []byte) error { +func (e *Etcd) CreateView(ctx context.Context, indexName, fieldName, name string) error { cli, err := e.client() if err != nil { return errors.Wrap(err, "CreateView: creating client") @@ -689,14 +689,10 @@ func (e *Etcd) CreateView(ctx context.Context, indexName, fieldName, name string key := schemaPrefix + indexName + "/" + fieldName + "/" + name - // Set up Op to write view value as bytes. - op := clientv3.OpPut(key, "") - op.WithValueBytes(val) - // Check for key existence, and execute Op within a transaction. _, err = cli.KV.Txn(ctx). If(clientv3util.KeyMissing(key)). - Then(op). + Then(clientv3.OpPut(key, "")). Commit() if err != nil { return errors.Wrap(err, "executing transaction") @@ -778,6 +774,23 @@ func (e *Etcd) getKey(ctx context.Context, key string) ([]string, [][]byte, erro return keys, values, nil } +func (e *Etcd) keyExists(ctx context.Context, key string) (bool, error) { + cli, err := e.client() + if err != nil { + return false, errors.Wrap(err, "keyExists: creates a new client") + } + defer cli.Close() + + resp, err := cli.Get(ctx, key, clientv3.WithCountOnly()) + if err != nil { + return false, err + } + if resp.Count > 0 { + return true, nil + } + return false, nil +} + func (e *Etcd) delKey(ctx context.Context, key string, withPrefix bool) error { cli, err := e.client() if err != nil { diff --git a/field.go b/field.go index 0bfdede5d..387456440 100644 --- a/field.go +++ b/field.go @@ -2241,10 +2241,5 @@ func (f *Field) persistView(ctx context.Context, cvm *CreateViewMessage) error { return ErrViewRequired } - if b, err := f.serializer.Marshal(cvm); err != nil { - return errors.Wrap(err, "marshaling") - } else if err := f.schemator.CreateView(ctx, cvm.Index, cvm.Field, cvm.View, b); err != nil { - return errors.Wrapf(err, "writing field to disco: %s/%s/%s", cvm.Index, cvm.Field, cvm.View) - } - return nil + return f.schemator.CreateView(ctx, cvm.Index, cvm.Field, cvm.View) } diff --git a/holder.go b/holder.go index c6c9977c7..0fdc64b49 100644 --- a/holder.go +++ b/holder.go @@ -911,12 +911,8 @@ func (h *Holder) schema(ctx context.Context, includeViews bool) ([]*IndexInfo, e Options: *cfm.Meta, } if includeViews { - for _, viewData := range field.Views { - cvm, err := h.decodeCreateViewMessage(viewData) - if err != nil { - return nil, errors.Wrap(err, "decoding CreateViewMessage") - } - fi.Views = append(fi.Views, &ViewInfo{Name: cvm.View}) + for viewName := range field.Views { + fi.Views = append(fi.Views, &ViewInfo{Name: viewName}) } sort.Sort(viewInfoSlice(fi.Views)) } @@ -1265,9 +1261,11 @@ func (h *Holder) loadField(indexName, fieldName string) (*Field, error) { } func (h *Holder) loadView(indexName, fieldName, viewName string) (*view, error) { - b, err := h.schemator.View(context.TODO(), indexName, fieldName, viewName) + b, err := h.schemator.View(context.Background(), indexName, fieldName, viewName) if err != nil { return nil, errors.Wrapf(err, "getting view: %s/%s/%s", indexName, fieldName, viewName) + } else if !b { + return nil, errors.Wrapf(err, "tried to load a nonexistent view: %s/%s/%s", indexName, fieldName, viewName) } // Get field. @@ -1276,20 +1274,7 @@ func (h *Holder) loadView(indexName, fieldName, viewName string) (*view, error) return nil, errors.Errorf("local field not found: %s/%s", indexName, fieldName) } - cvm, err := h.decodeCreateViewMessage(b) - if err != nil { - return nil, errors.Wrap(err, "decoding CreateFieldMessage") - } - - // I think we eventually want to get rid of storing the serialized view in - // etcd because all it keeps is the view name. So in that case we would always - // just use the viewName argument here. - vName := cvm.View - if fieldName == existenceFieldName { - vName = viewName - } - - return fld.createViewIfNotExists(vName) + return fld.createViewIfNotExists(viewName) } func (h *Holder) newIndex(path, name string) (*Index, error) { @@ -2315,11 +2300,3 @@ func (h *Holder) decodeCreateFieldMessage(b []byte) (*CreateFieldMessage, error) } return &cfm, nil } - -func (h *Holder) decodeCreateViewMessage(b []byte) (*CreateViewMessage, error) { - var cvm CreateViewMessage - if err := h.serializer.Unmarshal(b, &cvm); err != nil { - return nil, errors.Wrap(err, "unmarshaling") - } - return &cvm, nil -}