From 063a3b96e3772c453ae76e238104c608076f86d4 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Wed, 21 Oct 2020 23:44:55 -0500 Subject: [PATCH 1/2] Check index.keys before opening translate store --- index.go | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/index.go b/index.go index 260054f59..9cbe19fed 100644 --- a/index.go +++ b/index.go @@ -205,30 +205,32 @@ func (i *Index) open(withTimestamp bool) (err error) { return errors.Wrap(err, "opening attrstore") } - i.holder.Logger.Debugf("open translate store for index: %s", i.name) + if i.keys { + i.holder.Logger.Debugf("open translate store for index: %s", i.name) - var g errgroup.Group - var mu sync.Mutex - for partitionID := 0; partitionID < i.holder.partitionN; partitionID++ { - partitionID := partitionID + var g errgroup.Group + var mu sync.Mutex + for partitionID := 0; partitionID < i.holder.partitionN; partitionID++ { + partitionID := partitionID - g.Go(func() error { - store, err := i.OpenTranslateStore(i.TranslateStorePath(partitionID), i.name, "", partitionID, i.holder.partitionN) - if err != nil { - return errors.Wrapf(err, "opening index translate store: partition=%d", partitionID) - } + g.Go(func() error { + store, err := i.OpenTranslateStore(i.TranslateStorePath(partitionID), i.name, "", partitionID, i.holder.partitionN) + if err != nil { + return errors.Wrapf(err, "opening index translate store: partition=%d", partitionID) + } - mu.Lock() - defer mu.Unlock() + mu.Lock() + defer mu.Unlock() - i.mu.Lock() - defer i.mu.Unlock() - i.translateStores[partitionID] = store - return nil - }) - } - if err := g.Wait(); err != nil { - return err + i.mu.Lock() + defer i.mu.Unlock() + i.translateStores[partitionID] = store + return nil + }) + } + if err := g.Wait(); err != nil { + return err + } } _ = testhook.Opened(i.holder.Auditor, i, nil) From a48bf28be22fdfa291f5b550f5644bf3328658a7 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 20 Nov 2020 14:48:28 -0600 Subject: [PATCH 2/2] don't try to translate keys on unkeyed indexes this causes a few things to error earlier than they otherwise would have, hence the changed tests. --- cluster.go | 4 ++++ executor_test.go | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cluster.go b/cluster.go index 5eb4acc82..765ee7845 100644 --- a/cluster.go +++ b/cluster.go @@ -2918,6 +2918,10 @@ func (c *cluster) createIndexKeys(ctx context.Context, indexName string, keys .. return nil, ErrIndexNotFound } + if !idx.keys { + return nil, errors.Errorf("can't create index keys on unkeyed index %s", indexName) + } + // Split keys by partition. keysByPartition := make(map[int][]string, c.partitionN) for _, key := range keys { diff --git a/executor_test.go b/executor_test.go index 3d8cddd7f..29e843d6e 100644 --- a/executor_test.go +++ b/executor_test.go @@ -575,7 +575,7 @@ func TestExecutor_Execute_Set(t *testing.T) { }) t.Run("ErrInvalidColValueType", func(t *testing.T) { - if _, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set("foo", f=1)`}); err == nil || !hasCause(err, pilosa.ErrTranslatingKeyNotFound) || !strings.Contains(err.Error(), "unkeyed index") { + if _, err := cmd.API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set("foo", f=1)`}); err == nil || !strings.Contains(err.Error(), "unkeyed index") { t.Fatalf("The error is: '%v'", err) } }) @@ -977,7 +977,7 @@ func TestExecutor_Execute_SetValue(t *testing.T) { }) t.Run("ColumnBSIGroupValue", func(t *testing.T) { - if _, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set("bad_column", f=100)`}); err == nil || !hasCause(err, pilosa.ErrTranslatingKeyNotFound) || !strings.Contains(err.Error(), "unkeyed index") { + if _, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set("bad_column", f=100)`}); err == nil || !strings.Contains(err.Error(), "unkeyed index") { t.Fatalf("unexpected error: %s", err) } })