From 93fc4d09617c75b0d29e6cb5f87e48d6e181b202 Mon Sep 17 00:00:00 2001 From: arulajmani Date: Wed, 24 Feb 2021 11:17:14 -0500 Subject: [PATCH] sql: update database zone configurations after region drop finalization Previously, database level zone configurations weren't updated when the region drop was finalized. This patch adds that support. This patch also moves setting zone configurations on ADD REGION from the user txn to the type schema changer. This ensures that zone configurations are added transactionally -- if the ADD REGION fails for whatever reason, we no longer leave behind dangling zone config. Lastly, I've refactored some test setup code that was being duplicated and improved the tests around rollback to test additional scenarios. Closes #60435 Closes #60750 Release note: None Release justification: bug fixes and low-risk updates to new functionality --- .../testdata/logic_test/multi_region | 32 +++ .../testdata/logic_test/regional_by_row | 24 +-- pkg/ccl/multiregionccl/region_test.go | 189 ++++++++---------- .../multiregionccl/regional_by_row_test.go | 38 +--- pkg/sql/alter_database.go | 11 - pkg/sql/testutils.go | 42 ++++ pkg/sql/type_change.go | 39 +++- 7 files changed, 204 insertions(+), 171 deletions(-) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/multi_region b/pkg/ccl/logictestccl/testdata/logic_test/multi_region index 054ab3cd4119..62ce9c9674e2 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/multi_region +++ b/pkg/ccl/logictestccl/testdata/logic_test/multi_region @@ -1083,3 +1083,35 @@ ALTER TABLE drop_primary_regions_db.primary SET LOCALITY REGIONAL BY TABLE IN PR statement ok ALTER DATABASE drop_primary_regions_db DROP REGION "ca-central-1" + +statement ok +CREATE DATABASE zone_config_drop_region PRIMARY REGION "ca-central-1" REGIONS "us-east-1", "ap-southeast-2" + +query TT +SHOW ZONE CONFIGURATION FOR DATABASE zone_config_drop_region +---- +DATABASE zone_config_drop_region ALTER DATABASE zone_config_drop_region CONFIGURE ZONE USING + range_min_bytes = 134217728, + range_max_bytes = 536870912, + gc.ttlseconds = 90000, + num_replicas = 5, + num_voters = 3, + constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}', + voter_constraints = '[+region=ca-central-1]', + lease_preferences = '[[+region=ca-central-1]]' + +statement ok +ALTER DATABASE zone_config_drop_region DROP REGION "us-east-1" + +query TT +SHOW ZONE CONFIGURATION FOR DATABASE zone_config_drop_region +---- +DATABASE zone_config_drop_region ALTER DATABASE zone_config_drop_region CONFIGURE ZONE USING + range_min_bytes = 134217728, + range_max_bytes = 536870912, + gc.ttlseconds = 90000, + num_replicas = 4, + num_voters = 3, + constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1}', + voter_constraints = '[+region=ca-central-1]', + lease_preferences = '[[+region=ca-central-1]]' diff --git a/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row b/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row index 8209c741846e..c69255bbe197 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row +++ b/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row @@ -1657,9 +1657,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING range_min_bytes = 134217728, range_max_bytes = 536870912, gc.ttlseconds = 90000, - num_replicas = 5, + num_replicas = 4, num_voters = 3, - constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}', + constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1}', voter_constraints = '[+region=ca-central-1]', lease_preferences = '[[+region=ca-central-1]]' @@ -1699,9 +1699,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING range_min_bytes = 134217728, range_max_bytes = 536870912, gc.ttlseconds = 90000, - num_replicas = 5, + num_replicas = 4, num_voters = 3, - constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}', + constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1}', voter_constraints = '[+region=ca-central-1]', lease_preferences = '[[+region=ca-central-1]]' @@ -1749,9 +1749,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING range_min_bytes = 134217728, range_max_bytes = 536870912, gc.ttlseconds = 90000, - num_replicas = 5, + num_replicas = 4, num_voters = 3, - constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}', + constraints = '{+region=ca-central-1: 1, +region=us-east-1: 1}', voter_constraints = '[+region=ca-central-1]', lease_preferences = '[[+region=ca-central-1]]' @@ -1791,9 +1791,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING range_min_bytes = 134217728, range_max_bytes = 536870912, gc.ttlseconds = 90000, - num_replicas = 5, + num_replicas = 4, num_voters = 3, - constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}', + constraints = '{+region=ca-central-1: 1, +region=us-east-1: 1}', voter_constraints = '[+region=ca-central-1]', lease_preferences = '[[+region=ca-central-1]]' @@ -1831,9 +1831,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING range_min_bytes = 134217728, range_max_bytes = 536870912, gc.ttlseconds = 90000, - num_replicas = 5, + num_replicas = 3, num_voters = 3, - constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}', + constraints = '{+region=ca-central-1: 1}', voter_constraints = '[+region=ca-central-1]', lease_preferences = '[[+region=ca-central-1]]' @@ -1844,9 +1844,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING range_min_bytes = 134217728, range_max_bytes = 536870912, gc.ttlseconds = 90000, - num_replicas = 5, + num_replicas = 3, num_voters = 3, - constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}', + constraints = '{+region=ca-central-1: 1}', voter_constraints = '[+region=ca-central-1]', lease_preferences = '[[+region=ca-central-1]]' diff --git a/pkg/ccl/multiregionccl/region_test.go b/pkg/ccl/multiregionccl/region_test.go index bfbc4f54ec00..4f2f8a51db9c 100644 --- a/pkg/ccl/multiregionccl/region_test.go +++ b/pkg/ccl/multiregionccl/region_test.go @@ -9,17 +9,12 @@ package multiregionccl_test import ( - "context" - "fmt" "testing" "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/sqltestutils" - "github.com/cockroachdb/cockroach/pkg/sql/tests" "github.com/cockroachdb/cockroach/pkg/testutils" - "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/syncutil" @@ -32,47 +27,27 @@ func TestSettingPrimaryRegionAmidstDrop(t *testing.T) { defer log.Scope(t).Close(t) numServers := 2 - serverArgs := make(map[int]base.TestServerArgs) - - regionNames := make([]string, numServers) - for i := 0; i < numServers; i++ { - // "us-east1", "us-east2"... - regionNames[i] = fmt.Sprintf("us-east%d", i+1) - } var mu syncutil.Mutex dropRegionStarted := make(chan struct{}) waitForPrimaryRegionSwitch := make(chan struct{}) dropRegionFinished := make(chan struct{}) - for i := 0; i < numServers; i++ { - serverArgs[i] = base.TestServerArgs{ - Knobs: base.TestingKnobs{ - SQLTypeSchemaChanger: &sql.TypeSchemaChangerTestingKnobs{ - RunBeforeEnumMemberPromotion: func() { - mu.Lock() - defer mu.Unlock() - if dropRegionStarted != nil { - close(dropRegionStarted) - <-waitForPrimaryRegionSwitch - dropRegionStarted = nil - } - }, - }, + knobs := base.TestingKnobs{ + SQLTypeSchemaChanger: &sql.TypeSchemaChangerTestingKnobs{ + RunBeforeEnumMemberPromotion: func() { + mu.Lock() + defer mu.Unlock() + if dropRegionStarted != nil { + close(dropRegionStarted) + <-waitForPrimaryRegionSwitch + dropRegionStarted = nil + } }, - Locality: roachpb.Locality{ - Tiers: []roachpb.Tier{{Key: "region", Value: regionNames[i]}}, - }, - } + }, } - tc := serverutils.StartNewTestCluster(t, numServers, base.TestClusterArgs{ - ServerArgsPerNode: serverArgs, - }) - - ctx := context.Background() - defer tc.Stopper().Stop(ctx) - - sqlDB := tc.ServerConn(0) + _, sqlDB, cleanup := sql.CreateTestMultiRegionCluster(t, numServers, knobs) + defer cleanup() // Setup the test. _, err := sqlDB.Exec(`CREATE DATABASE db WITH PRIMARY REGION "us-east1" REGIONS "us-east2"`) @@ -142,44 +117,41 @@ func TestDroppingPrimaryRegionAsyncJobFailure(t *testing.T) { // Decrease the adopt loop interval so that retries happen quickly. defer sqltestutils.SetTestJobsAdoptInterval()() - params, _ := tests.CreateTestServerParams() - params.Locality.Tiers = []roachpb.Tier{ - {Key: "region", Value: "us-east-1"}, - } - // Protects expectedCleanupRuns var mu syncutil.Mutex // We need to cleanup 2 times, once for the multi-region type descriptor and // once for the array alias of the multi-region type descriptor. haveWePerformedFirstRoundOfCleanup := false cleanupFinished := make(chan struct{}) - params.Knobs.SQLTypeSchemaChanger = &sql.TypeSchemaChangerTestingKnobs{ - RunBeforeExec: func() error { - return errors.New("yikes") - }, - RunAfterOnFailOrCancel: func() error { - mu.Lock() - defer mu.Unlock() - if haveWePerformedFirstRoundOfCleanup { - close(cleanupFinished) - } - haveWePerformedFirstRoundOfCleanup = true - return nil + knobs := base.TestingKnobs{ + SQLTypeSchemaChanger: &sql.TypeSchemaChangerTestingKnobs{ + RunBeforeExec: func() error { + return errors.New("yikes") + }, + RunAfterOnFailOrCancel: func() error { + mu.Lock() + defer mu.Unlock() + if haveWePerformedFirstRoundOfCleanup { + close(cleanupFinished) + } + haveWePerformedFirstRoundOfCleanup = true + return nil + }, }, } - s, sqlDB, _ := serverutils.StartServer(t, params) - ctx := context.Background() - defer s.Stopper().Stop(ctx) + numServers := 1 + _, sqlDB, cleanup := sql.CreateTestMultiRegionCluster(t, numServers, knobs) + defer cleanup() // Setup the test. _, err := sqlDB.Exec(` -CREATE DATABASE db WITH PRIMARY REGION "us-east-1"; +CREATE DATABASE db WITH PRIMARY REGION "us-east1"; CREATE TABLE db.t(k INT) LOCALITY REGIONAL BY TABLE IN PRIMARY REGION; `) require.NoError(t, err) - _, err = sqlDB.Exec(`ALTER DATABASE db DROP REGION "us-east-1"`) + _, err = sqlDB.Exec(`ALTER DATABASE db DROP REGION "us-east1"`) testutils.IsError(err, "yikes") <-cleanupFinished @@ -192,7 +164,7 @@ CREATE TABLE db.t(k INT) LOCALITY REGIONAL BY TABLE IN PRIMARY REGION; t.Fatal("expected crdb_internal_region not to be present in system.namespace") } - _, err = sqlDB.Exec(`ALTER DATABASE db PRIMARY REGION "us-east-1"`) + _, err = sqlDB.Exec(`ALTER DATABASE db PRIMARY REGION "us-east1"`) require.NoError(t, err) rows = sqlDB.QueryRow(`SELECT count(*) FROM system.namespace WHERE name = 'crdb_internal_region'`) @@ -203,61 +175,68 @@ CREATE TABLE db.t(k INT) LOCALITY REGIONAL BY TABLE IN PRIMARY REGION; } } -// TODO(arul): Currently, database level zone configurations aren't rolled back. -// When we fix this limitation we should also update this test to check for that. -func TestRollbackDuringAddRegion(t *testing.T) { +// TestRollbackDuringAddDropRegionAsyncJobFailure ensures that rollback when +// an ADD REGION/DROP REGION fails asynchronously is handled appropriately. +// This also ensures that zone configuration changes on the database are +// transactional. +func TestRollbackDuringAddDropRegionAsyncJobFailure(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) // Decrease the adopt loop interval so that retries happen quickly. defer sqltestutils.SetTestJobsAdoptInterval()() - numServers := 2 - serverArgs := make(map[int]base.TestServerArgs) - - regionNames := make([]string, numServers) - for i := 0; i < numServers; i++ { - // "us-east1", "us-east2"... - regionNames[i] = fmt.Sprintf("us-east%d", i+1) - } - - for i := 0; i < numServers; i++ { - serverArgs[i] = base.TestServerArgs{ - Knobs: base.TestingKnobs{ - SQLTypeSchemaChanger: &sql.TypeSchemaChangerTestingKnobs{ - RunBeforeExec: func() error { - return errors.New("boom") - }, - }, + numServers := 3 + knobs := base.TestingKnobs{ + SQLTypeSchemaChanger: &sql.TypeSchemaChangerTestingKnobs{ + RunBeforeMultiRegionUpdates: func() error { + return errors.New("boom") }, - Locality: roachpb.Locality{ - Tiers: []roachpb.Tier{{Key: "region", Value: regionNames[i]}}, - }, - } + }, } - tc := serverutils.StartNewTestCluster(t, numServers, base.TestClusterArgs{ - ServerArgsPerNode: serverArgs, - }) - - ctx := context.Background() - defer tc.Stopper().Stop(ctx) + // Setup + _, sqlDB, cleanup := sql.CreateTestMultiRegionCluster(t, numServers, knobs) + defer cleanup() - sqlDB := tc.ServerConn(0) - - _, err := sqlDB.Exec(`CREATE DATABASE db WITH PRIMARY REGION "us-east1"`) + _, err := sqlDB.Exec(`CREATE DATABASE db WITH PRIMARY REGION "us-east1" REGIONS "us-east2"`) require.NoError(t, err) - if err != nil { - t.Error(err) - } - _, err = sqlDB.Exec(`ALTER DATABASE db ADD REGION "us-east2"`) - require.Error(t, err, "boom") + testCases := []struct { + query string + }{ + {`ALTER DATABASE db ADD REGION "us-east3"`}, + {`ALTER DATABASE db DROP REGION "us-east2"`}, + {`BEGIN; ALTER DATABASE db DROP REGION "us-east2"; ALTER DATABASE db ADD REGION "us-east3"; COMMIT`}, + } - var jobStatus string - var jobErr string - row := sqlDB.QueryRow("SELECT status, error FROM [SHOW JOBS] WHERE job_type = 'TYPEDESC SCHEMA CHANGE'") - require.NoError(t, row.Scan(&jobStatus, &jobErr)) - require.Regexp(t, "boom", jobErr) - require.Regexp(t, "failed", jobStatus) + for _, tc := range testCases { + var originalZoneConfig string + res := sqlDB.QueryRow(`SELECT raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR DATABASE db]`) + err = res.Scan(&originalZoneConfig) + require.NoError(t, err) + + _, err = sqlDB.Exec(tc.query) + testutils.IsError(err, "boom") + + var jobStatus string + var jobErr string + row := sqlDB.QueryRow("SELECT status, error FROM [SHOW JOBS] WHERE job_type = 'TYPEDESC SCHEMA CHANGE'") + require.NoError(t, row.Scan(&jobStatus, &jobErr)) + require.Regexp(t, "boom", jobErr) + require.Regexp(t, "failed", jobStatus) + + // Ensure the zone configuration didn't change + var newZoneConfig string + res = sqlDB.QueryRow(`SELECT raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR DATABASE db]`) + err = res.Scan(&newZoneConfig) + require.NoError(t, err) + + if newZoneConfig != originalZoneConfig { + t.Fatalf("expected zone config to not have changed, expected %q found %q", + originalZoneConfig, + newZoneConfig, + ) + } + } } diff --git a/pkg/ccl/multiregionccl/regional_by_row_test.go b/pkg/ccl/multiregionccl/regional_by_row_test.go index 17f6005b7098..313ab017912f 100644 --- a/pkg/ccl/multiregionccl/regional_by_row_test.go +++ b/pkg/ccl/multiregionccl/regional_by_row_test.go @@ -36,42 +36,6 @@ import ( // REGIONAL BY ROW tests are defined in multiregionccl as REGIONAL BY ROW // requires CCL to operate. -// createTestMultiRegionCluster creates a test cluster with numServers number of -// nodes with the provided testing knobs applied to each of the nodes. Every -// node is placed in its own locality, named "us-east1", "us-east2", and so on. -func createTestMultiRegionCluster( - t *testing.T, numServers int, knobs base.TestingKnobs, -) (serverutils.TestClusterInterface, *gosql.DB, func()) { - serverArgs := make(map[int]base.TestServerArgs) - regionNames := make([]string, numServers) - for i := 0; i < numServers; i++ { - // "us-east1", "us-east2"... - regionNames[i] = fmt.Sprintf("us-east%d", i+1) - } - - for i := 0; i < numServers; i++ { - serverArgs[i] = base.TestServerArgs{ - Knobs: knobs, - Locality: roachpb.Locality{ - Tiers: []roachpb.Tier{{Key: "region", Value: regionNames[i]}}, - }, - } - } - - tc := serverutils.StartNewTestCluster(t, numServers, base.TestClusterArgs{ - ServerArgsPerNode: serverArgs, - }) - - ctx := context.Background() - cleanup := func() { - tc.Stopper().Stop(ctx) - } - - sqlDB := tc.ServerConn(0) - - return tc, sqlDB, cleanup -} - // TestAlterTableLocalityRegionalByRowError tests an alteration involving // REGIONAL BY ROW which gets its async job interrupted by some sort of // error or cancellation. After this, we expect the table to retain @@ -417,7 +381,7 @@ func TestRepartitionFailureRollback(t *testing.T) { }, }, } - _, sqlDB, cleanup := createTestMultiRegionCluster(t, numServers, knobs) + _, sqlDB, cleanup := sql.CreateTestMultiRegionCluster(t, numServers, knobs) defer cleanup() _, err := sqlDB.Exec( diff --git a/pkg/sql/alter_database.go b/pkg/sql/alter_database.go index 46015702db79..6b4e92d1df02 100644 --- a/pkg/sql/alter_database.go +++ b/pkg/sql/alter_database.go @@ -227,17 +227,6 @@ func (n *alterDatabaseAddRegionNode) startExec(params runParams) error { return err } - // Update the database's zone configuration. - if err := ApplyZoneConfigFromDatabaseRegionConfig( - params.ctx, - n.desc.ID, - *n.desc.RegionConfig, - params.p.txn, - params.p.execCfg, - ); err != nil { - return err - } - // Log Alter Database Add Region event. This is an auditable log event and is // recorded in the same transaction as the database descriptor, type // descriptor, and zone configuration updates. diff --git a/pkg/sql/testutils.go b/pkg/sql/testutils.go index 1a1df55b1b55..e419c9026acb 100644 --- a/pkg/sql/testutils.go +++ b/pkg/sql/testutils.go @@ -12,14 +12,20 @@ package sql import ( "context" + gosql "database/sql" + "fmt" + "testing" + "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/errors" ) @@ -148,3 +154,39 @@ func (dsp *DistSQLPlanner) Exec( dsp.PlanAndRun(ctx, evalCtx, planCtx, p.txn, p.curPlan.main, recv)() return rw.Err() } + +// CreateTestMultiRegionCluster creates a test cluster with numServers number of +// nodes with the provided testing knobs applied to each of the nodes. Every +// node is placed in its own locality, named "us-east1", "us-east2", and so on. +func CreateTestMultiRegionCluster( + t *testing.T, numServers int, knobs base.TestingKnobs, +) (serverutils.TestClusterInterface, *gosql.DB, func()) { + serverArgs := make(map[int]base.TestServerArgs) + regionNames := make([]string, numServers) + for i := 0; i < numServers; i++ { + // "us-east1", "us-east2"... + regionNames[i] = fmt.Sprintf("us-east%d", i+1) + } + + for i := 0; i < numServers; i++ { + serverArgs[i] = base.TestServerArgs{ + Knobs: knobs, + Locality: roachpb.Locality{ + Tiers: []roachpb.Tier{{Key: "region", Value: regionNames[i]}}, + }, + } + } + + tc := serverutils.StartNewTestCluster(t, numServers, base.TestClusterArgs{ + ServerArgsPerNode: serverArgs, + }) + + ctx := context.Background() + cleanup := func() { + tc.Stopper().Stop(ctx) + } + + sqlDB := tc.ServerConn(0) + + return tc, sqlDB, cleanup +} diff --git a/pkg/sql/type_change.go b/pkg/sql/type_change.go index 6288eb1dfff8..5923e18f096a 100644 --- a/pkg/sql/type_change.go +++ b/pkg/sql/type_change.go @@ -25,6 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkv" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" @@ -306,7 +307,7 @@ func (t *typeSchemaChanger) exec(ctx context.Context) error { if fn := t.execCfg.TypeSchemaChangerTestingKnobs.RunBeforeMultiRegionUpdates; fn != nil { return fn() } - repartitionedTables, err = repartitionRegionalByRowTables( + repartitionedTables, err = performMultiRegionFinalization( ctx, immut, txn, @@ -366,6 +367,36 @@ func (t *typeSchemaChanger) exec(ctx context.Context) error { return nil } +// performMultiRegionFinalization updates the zone configurations on the +// database and re-partitions all REGIONAL BY ROW tables after REGION ADD/DROP +// has completed. A list of re-partitioned tables, if any, is returned. +func performMultiRegionFinalization( + ctx context.Context, + typeDesc *typedesc.Immutable, + txn *kv.Txn, + execCfg *ExecutorConfig, + descsCol *descs.Collection, +) ([]descpb.ID, error) { + _, dbDesc, err := descsCol.GetImmutableDatabaseByID( + ctx, txn, typeDesc.ParentID, tree.DatabaseLookupFlags{Required: true}) + if err != nil { + return nil, err + } + // Once the region promotion/demotion is complete, we update the + // zone configuration on the database. + if err := ApplyZoneConfigFromDatabaseRegionConfig( + ctx, + dbDesc.ID, + *dbDesc.RegionConfig, + txn, + execCfg, + ); err != nil { + return nil, err + } + + return repartitionRegionalByRowTables(ctx, typeDesc, dbDesc, txn, execCfg, descsCol) +} + // repartitionRegionalByRowTables takes a multi-region enum and re-partitions // all REGIONAL BY ROW tables in the enclosing database such that there is a // partition and corresponding zone configuration for all PUBLIC enum members @@ -378,6 +409,7 @@ func (t *typeSchemaChanger) exec(ctx context.Context) error { func repartitionRegionalByRowTables( ctx context.Context, typeDesc *typedesc.Immutable, + dbDesc *dbdesc.Immutable, txn *kv.Txn, execCfg *ExecutorConfig, descsCol *descs.Collection, @@ -400,11 +432,6 @@ func repartitionRegionalByRowTables( defer cleanup() localPlanner := p.(*planner) - _, dbDesc, err := localPlanner.Descriptors().GetImmutableDatabaseByID( - ctx, txn, typeDesc.ParentID, tree.DatabaseLookupFlags{Required: true}) - if err != nil { - return nil, err - } allDescs, err := localPlanner.Descriptors().GetAllDescriptors(ctx, txn) if err != nil { return nil, err