diff --git a/pkg/ccl/backupccl/testdata/backup-restore/multiregion b/pkg/ccl/backupccl/testdata/backup-restore/multiregion index 94435ec36be2..934e3b93cd58 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/multiregion +++ b/pkg/ccl/backupccl/testdata/backup-restore/multiregion @@ -69,10 +69,24 @@ exec-sql RESTORE FROM LATEST IN 'nodelocal://1/full_cluster_backup/' WITH skip_localities_check; ---- +exec-sql +ALTER DATABASE d SET PRIMARY REGION 'eu-central-1'; +ALTER DATABASE d DROP REGION 'us-east-1'; +ALTER DATABASE d DROP REGION 'us-west-1'; +ALTER DATABASE d ADD REGION 'eu-north-1'; +---- + exec-sql RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH skip_localities_check, new_db_name='d_new'; ---- +exec-sql +ALTER DATABASE d_new SET PRIMARY REGION 'eu-central-1'; +ALTER DATABASE d_new DROP REGION 'us-east-1'; +ALTER DATABASE d_new DROP REGION 'us-west-1'; +ALTER DATABASE d_new ADD REGION 'eu-north-1'; +---- + exec-sql DROP DATABASE d_new; ---- diff --git a/pkg/sql/alter_database.go b/pkg/sql/alter_database.go index 99772b5d12d2..bcaf0e29ac22 100644 --- a/pkg/sql/alter_database.go +++ b/pkg/sql/alter_database.go @@ -897,6 +897,10 @@ func (n *alterDatabasePrimaryRegionNode) switchPrimaryRegion(params runParams) e return err } + // Validate the final zone config at the end of the transaction, since + // we will not be validating localities right now. + *params.extendedEvalCtx.validateDbZoneConfig = true + // Update the database's zone configuration. if err := ApplyZoneConfigFromDatabaseRegionConfig( params.ctx, @@ -904,7 +908,7 @@ func (n *alterDatabasePrimaryRegionNode) switchPrimaryRegion(params runParams) e updatedRegionConfig, params.p.InternalSQLTxn(), params.p.execCfg, - true, /* validateLocalities */ + false, /*validateLocalities*/ params.extendedEvalCtx.Tracing.KVTracingEnabled(), ); err != nil { return err @@ -2021,6 +2025,8 @@ func (n *alterDatabaseSecondaryRegion) startExec(params runParams) error { return err } + *params.extendedEvalCtx.validateDbZoneConfig = true + // Update the database's zone configuration. if err := ApplyZoneConfigFromDatabaseRegionConfig( params.ctx, diff --git a/pkg/sql/catalog/descs/collection.go b/pkg/sql/catalog/descs/collection.go index 3f4af2020898..c9177928653c 100644 --- a/pkg/sql/catalog/descs/collection.go +++ b/pkg/sql/catalog/descs/collection.go @@ -702,6 +702,18 @@ func (tc *Collection) GetUncommittedTables() (tables []catalog.TableDescriptor) return tables } +// GetUncommittedDatabases returns all the databases updated or created in the +// transaction. +func (tc *Collection) GetUncommittedDatabases() (databases []catalog.DatabaseDescriptor) { + _ = tc.uncommitted.iterateUncommittedByID(func(desc catalog.Descriptor) error { + if database, ok := desc.(catalog.DatabaseDescriptor); ok { + databases = append(databases, database) + } + return nil + }) + return databases +} + func newMutableSyntheticDescriptorAssertionError(id descpb.ID) error { return errors.AssertionFailedf("attempted mutable access of synthetic descriptor %d", id) } diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go index b54ed4fde584..568c9a7475e1 100644 --- a/pkg/sql/conn_executor.go +++ b/pkg/sql/conn_executor.go @@ -1357,6 +1357,9 @@ type connExecutor struct { // comprising statements. numRows int + // validateDbZoneConfig should the DB zone config on commit. + validateDbZoneConfig bool + // txnCounter keeps track of how many SQL txns have been open since // the start of the session. This is used for logging, to // distinguish statements that belong to separate SQL transactions. @@ -1914,6 +1917,7 @@ func (ex *connExecutor) resetExtraTxnState(ctx context.Context, ev txnEvent) { } else { ex.extraTxnState.descCollection.ReleaseAll(ctx) ex.extraTxnState.jobs.reset() + ex.extraTxnState.validateDbZoneConfig = false ex.extraTxnState.schemaChangerState.memAcc.Clear(ctx) ex.extraTxnState.schemaChangerState = &SchemaChangerState{ mode: ex.sessionData().NewSchemaChangerMode, @@ -3403,14 +3407,15 @@ func (ex *connExecutor) initEvalCtx(ctx context.Context, evalCtx *extendedEvalCo RangeStatsFetcher: p.execCfg.RangeStatsFetcher, JobsProfiler: p, }, - Tracing: &ex.sessionTracing, - MemMetrics: &ex.memMetrics, - Descs: ex.extraTxnState.descCollection, - TxnModesSetter: ex, - jobs: ex.extraTxnState.jobs, - statsProvider: ex.server.sqlStats, - indexUsageStats: ex.indexUsageStats, - statementPreparer: ex, + Tracing: &ex.sessionTracing, + MemMetrics: &ex.memMetrics, + Descs: ex.extraTxnState.descCollection, + TxnModesSetter: ex, + jobs: ex.extraTxnState.jobs, + validateDbZoneConfig: &ex.extraTxnState.validateDbZoneConfig, + statsProvider: ex.server.sqlStats, + indexUsageStats: ex.indexUsageStats, + statementPreparer: ex, } evalCtx.copyFromExecCfg(ex.server.cfg) } diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index 32968afd3105..4f94edf900dc 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -1311,6 +1311,10 @@ func (ex *connExecutor) commitSQLTransactionInternal(ctx context.Context) error ex.state.mu.txn.ConfigureStepping(ctx, prevSteppingMode) } + if err := ex.validateDbZoneConfigs(ctx); err != nil { + return err + } + if err := ex.createJobs(ctx); err != nil { return err } @@ -1356,6 +1360,33 @@ func (ex *connExecutor) commitSQLTransactionInternal(ctx context.Context) error return nil } +// validateDbZoneConfigs validates zone configs for any modified databases. +func (ex *connExecutor) validateDbZoneConfigs(ctx context.Context) error { + // If no DDL was executed requiring pre-commit validation, + // then nothing to do. + if !ex.extraTxnState.validateDbZoneConfig { + return nil + } + for _, db := range ex.extraTxnState.descCollection.GetUncommittedDatabases() { + regionConfig, err := SynthesizeRegionConfig( + ctx, ex.planner.txn, db.GetID(), ex.planner.Descriptors(), + ) + if err != nil { + return err + } + _, err = generateAndValidateZoneConfigForMultiRegionDatabase(ctx, + ex.planner.regionsProvider(), + ex.planner.execCfg, + regionConfig, + true, /*validateLocalities*/ + ) + if err != nil { + return err + } + } + return nil +} + // createJobs creates jobs for the records cached in schemaChangeJobRecords // during this transaction. func (ex *connExecutor) createJobs(ctx context.Context) error { diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index b19dec4c4511..1f771cd7c498 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -109,6 +109,9 @@ type extendedEvalContext struct { SchemaChangerState *SchemaChangerState statementPreparer statementPreparer + + // validateDbZoneConfig should the DB zone config on commit. + validateDbZoneConfig *bool } // copyFromExecCfg copies relevant fields from an ExecutorConfig.