From 3f586463de7fc5b5a8148da2b9446c796b2fd3e1 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Tue, 23 Mar 2021 02:00:50 -0400 Subject: [PATCH] partitionccl: do not generate subzone spans for dropped tables This commit fixes a hazard posed by tables partitioned by user-defined types. The bug being fixed involves code running in an index GC job which attempts to re-write zone configs on a dropped table which was partitioned by a now-dropped user-defined type. Before this change, we'd attempt to generate subzone spans which we might not know how to populate. Release note (bug fix): Fixed a bug from earlier alphas in 21.1 whereby dropping an index on a table partitioned by a user-defined type and then dropping the table and then dropping the type before the GC TTL for the index has expired can result in a crash. --- pkg/ccl/partitionccl/drop_test.go | 53 +++++++++++++++++++++++++++++++ pkg/sql/partition_utils.go | 9 ++++++ 2 files changed, 62 insertions(+) diff --git a/pkg/ccl/partitionccl/drop_test.go b/pkg/ccl/partitionccl/drop_test.go index 144846f8c864..87b975a5167f 100644 --- a/pkg/ccl/partitionccl/drop_test.go +++ b/pkg/ccl/partitionccl/drop_test.go @@ -211,4 +211,57 @@ SELECT job_id waitForJobDone(t, tdb, "GC for DROP INDEX%idx") }) + + // This is a regression test for a hazardous scenario whereby a drop index gc + // job may attempt to rewrite subzone spans for a dropped table which used types + // which no longer exist. + t.Run("drop table and type", func(t *testing.T) { + + // Sketch of the test: + // + // * Set up a partitioned table and index which are partitioned by an enum. + // * Set a short GC TTL on the index. + // * Drop the index. + // * Drop the table. + // * Drop the type. + // * Wait for the index to be cleaned up, which would have crashed before the + // this fix. + // * Set a short GC TTL on everything. + // * Wait for the table to be cleaned up. + // + + defer log.Scope(t).Close(t) + ctx := context.Background() + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{}) + defer tc.Stopper().Stop(ctx) + + tdb := sqlutils.MakeSQLRunner(tc.ServerConn(0)) + + tdb.Exec(t, ` + CREATE TYPE typ AS ENUM ('a', 'b', 'c'); + CREATE TABLE t (e typ PRIMARY KEY) PARTITION BY LIST (e) ( + PARTITION a VALUES IN ('a'), + PARTITION b VALUES IN ('b'), + PARTITION c VALUES IN ('c') + ); + CREATE INDEX idx + ON t (e) + PARTITION BY LIST (e) + ( + PARTITION ai VALUES IN ('a'), + PARTITION bi VALUES IN ('b'), + PARTITION ci VALUES IN ('c') + ); + ALTER PARTITION ai OF INDEX t@idx CONFIGURE ZONE USING range_min_bytes = 123456, range_max_bytes = 654321; + ALTER PARTITION a OF TABLE t CONFIGURE ZONE USING range_min_bytes = 123456, range_max_bytes = 654321; + ALTER INDEX t@idx CONFIGURE ZONE USING gc.ttlseconds = 1; + DROP INDEX t@idx; + DROP TABLE t; + DROP TYPE typ; + `) + + waitForJobDone(t, tdb, "GC for DROP INDEX%idx") + tdb.Exec(t, `ALTER RANGE default CONFIGURE ZONE USING gc.ttlseconds = 1`) + waitForJobDone(t, tdb, "GC for DROP TABLE%t") + }) } diff --git a/pkg/sql/partition_utils.go b/pkg/sql/partition_utils.go index f354a28c7e2b..4dfe5626f096 100644 --- a/pkg/sql/partition_utils.go +++ b/pkg/sql/partition_utils.go @@ -86,6 +86,15 @@ func GenerateSubzoneSpans( } } + // We already completely avoid creating subzone spans for dropped indexes. + // Whether this was intentional is a different story, but it turns out to be + // pretty sane. Dropped elements may refer to dropped types and we aren't + // necessarily in a position to deal with those dropped types. Add a special + // case to avoid generating any subzone spans in the face of being dropped. + if tableDesc.Dropped() { + return nil, nil + } + a := &rowenc.DatumAlloc{} subzoneIndexByIndexID := make(map[descpb.IndexID]int32)