diff --git a/dbshard.go b/dbshard.go index 606fe75f2..a86cc4ab7 100644 --- a/dbshard.go +++ b/dbshard.go @@ -1228,6 +1228,10 @@ func (vs *FieldView2Shards) String() (r string) { return } +func (vs *FieldView2Shards) removeField(name string) { + delete(vs.m, name) +} + // Note: cannot call this during migration, because // it only ever returns the green shards if we are in blue-green. func (per *DBPerShard) GetFieldView2ShardsMapForIndex(idx *Index) (vs *FieldView2Shards, err error) { diff --git a/index.go b/index.go index 83a4e8e4a..4cfd301b2 100644 --- a/index.go +++ b/index.go @@ -805,6 +805,9 @@ func (i *Index) DeleteField(name string) error { // Remove reference. delete(i.fields, name) + // remove shard metadata for field + i.fieldView2shard.removeField(name) + // Delete the field from etcd as the system of record. 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/index_test.go b/index_test.go index 2837d8409..34fedc098 100644 --- a/index_test.go +++ b/index_test.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "math/rand" + "os" "reflect" "testing" "time" @@ -281,6 +282,10 @@ func isNotFoundError(err error) bool { return ok } +// Ensure that after node/cluster restart, deleting and recreating a field +// does not cause a deadlock +// This is a regression test after a customer experienced the same deadlock. +// For details, check out https://molecula.atlassian.net/browse/CORE-919 func TestIndex_RecreateFieldOnRestart(t *testing.T) { c := test.MustRunCluster(t, 1) defer c.Close() @@ -296,7 +301,7 @@ func TestIndex_RecreateFieldOnRestart(t *testing.T) { } defer index.Close() - // // create field + // create field fieldName := fmt.Sprintf("field_%d", rand.Uint64()) _, err = c.GetNode(0).API.CreateField(context.Background(), indexName, fieldName, pilosa.OptFieldTypeDefault()) @@ -337,8 +342,9 @@ func TestIndex_RecreateFieldOnRestart(t *testing.T) { }() select { case <-time.After(10 * time.Second): - t.Fatalf("recreating field took too long") - case <-errCh: + t.Logf("recreating field took too long") + os.Exit(1) + case err := <-errCh: if err != nil { t.Fatal(err) }