From 042aed3d40b93894d86eb85a0729a6e235f5f192 Mon Sep 17 00:00:00 2001 From: elliebababa Date: Tue, 11 May 2021 12:49:41 -0700 Subject: [PATCH] backupccl: allow restoring into multi-region databases Previously, restoring a non-multi-region table in to multi-region database is not allowed. This patch unblocks restoring non-multi-region table into multi-region database and sets the table locality as regional by table to the primary region of the target database. Additionally, GLOBAL tables, REGIONAL BY TABLE IN PRIMARY REGION and REGIONAL BY TABLE IN tables are also supported given that the is compatible with the target database. Release note (enterprise change): RESTORE now supports restoring individual tables into a multi-region database. If the table being restored is also multi-region, REGIONAL BY ROW tables cannot be restored, and REGIONAL BY TABLE tables can only be restored if their localities match those of the database they're being restored into. --- pkg/ccl/backupccl/restore_planning.go | 21 +- pkg/ccl/backupccl/targets.go | 70 +++++++ .../testdata/logic_test/multi_region_backup | 195 +++++++++++++++++- 3 files changed, 271 insertions(+), 15 deletions(-) diff --git a/pkg/ccl/backupccl/restore_planning.go b/pkg/ccl/backupccl/restore_planning.go index e3cca9a64b63..66075f433737 100644 --- a/pkg/ccl/backupccl/restore_planning.go +++ b/pkg/ccl/backupccl/restore_planning.go @@ -654,21 +654,14 @@ func allocateDescriptorRewrites( return err } - // We're restoring a table and not its parent database. If the - // new database we're placing the table in is a multi-region database, - // block the restore. We do this because we currently have no way to - // modify this table and make it multi-region friendly. Long-term we'd - // want to modify the table so that it can exist in the multi-region - // database. - // https://github.com/cockroachdb/cockroach/issues/59804 - if parentDB.IsMultiRegion() { - return pgerror.Newf(pgcode.FeatureNotSupported, - "cannot restore individual table %d into multi-region database %d", - table.GetID(), - parentDB.GetID(), - ) + if parentDB.IsMultiRegion() && table.GetLocalityConfig() != nil { + // We're restoring a table and not its parent database. We may block + // restoring multi-region tables to multi-region databases since regions + // may mismatch. + if err := checkMultiRegionCompatible(ctx, txn, p.ExecCfg().Codec, table, parentDB); err != nil { + return pgerror.WithCandidateCode(err, pgcode.FeatureNotSupported) + } } - // Create the table rewrite with the new parent ID. We've done all the // up-front validation that we can. descriptorRewrites[table.ID] = &jobspb.RestoreDetails_DescriptorRewrite{ParentID: parentID} diff --git a/pkg/ccl/backupccl/targets.go b/pkg/ccl/backupccl/targets.go index 4d4f30681928..798978777988 100644 --- a/pkg/ccl/backupccl/targets.go +++ b/pkg/ccl/backupccl/targets.go @@ -10,6 +10,7 @@ package backupccl import ( "context" + "fmt" "reflect" "sort" "strings" @@ -32,6 +33,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" @@ -612,3 +614,71 @@ func findLastSchemaChangeTime( } return lastSchemaChangeTime } + +// checkMultiRegionCompatible checks if the given table is compatible to be +// restored into the given database according to its multi-region locality. +// It returns an error describing the incompatibility if not. +func checkMultiRegionCompatible( + ctx context.Context, + txn *kv.Txn, + codec keys.SQLCodec, + table *tabledesc.Mutable, + database catalog.DatabaseDescriptor, +) error { + // If either the database or table are non-MR then allow it. + if !database.IsMultiRegion() || table.GetLocalityConfig() == nil { + return nil + } + + if table.IsLocalityGlobal() { + // Global tables are allowed because they do not reference a particular + // region. + return nil + } + + if table.IsLocalityRegionalByTable() { + regionName, _ := table.GetRegionalByTableRegion() + if regionName == descpb.RegionName(tree.PrimaryRegionNotSpecifiedName) { + // REGIONAL BY PRIMARY REGION tables are allowed since they do not + // reference a particular region. + return nil + } + + // For REGION BY TABLE IN tables, allow the restore if the + // database has the region. + regionEnumID := database.GetRegionConfig().RegionEnumID + regionEnum, err := catalogkv.MustGetTypeDescByID(ctx, txn, codec, regionEnumID) + if err != nil { + return err + } + dbRegionNames, err := regionEnum.RegionNames() + if err != nil { + return err + } + existingRegions := make([]string, len(dbRegionNames)) + for i, dbRegionName := range dbRegionNames { + if dbRegionName == regionName { + return nil + } + existingRegions[i] = fmt.Sprintf("%q", dbRegionName) + } + + return errors.Newf( + "cannot restore REGIONAL BY TABLE %s IN REGION %q (table ID: %d) into database %q; region %q not found in database regions %s", + table.GetName(), regionName, table.GetID(), + database.GetName(), regionName, strings.Join(existingRegions, ", "), + ) + } + + if table.IsLocalityRegionalByRow() { + return unimplemented.NewWithIssuef(67269, + "cannot restore REGIONAL BY ROW table %s (ID: %d) individually into a multi-region database %s", + table.GetName(), table.GetID(), database.GetName(), + ) + } + + return errors.AssertionFailedf( + "locality config of table %s (ID: %d) has locality %v which is unknown by RESTORE", + table.GetName(), table.GetID(), table.GetLocalityConfig(), + ) +} diff --git a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup index b1d896e0b7c3..0e08d1360fa7 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup +++ b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup @@ -204,6 +204,13 @@ CREATE TABLE regional_by_table_in_ca_central_1 ( FAMILY (pk, i) ) LOCALITY REGIONAL BY TABLE IN "ca-central-1" +statement ok +CREATE TABLE regional_by_table_in_ap_southeast_2 ( + pk INT PRIMARY KEY, + i INT, + FAMILY (pk, i) +) LOCALITY REGIONAL BY TABLE IN "ap-southeast-2" + query TTT SELECT partition_name, index_name, zone_config FROM [SHOW PARTITIONS FROM TABLE global_table] ORDER BY partition_name, index_name @@ -1025,8 +1032,194 @@ USE non_mr_backup statement ok CREATE TABLE non_mr_table (i int) +# For comparing the zone configurations before restore. +query TT +SHOW ZONE CONFIGURATION FROM TABLE non_mr_table +---- +RANGE default ALTER RANGE default CONFIGURE ZONE USING + range_min_bytes = 134217728, + range_max_bytes = 536870912, + gc.ttlseconds = 90000, + num_replicas = 3, + constraints = '[]', + lease_preferences = '[]' + statement ok BACKUP TABLE non_mr_table TO 'nodelocal://1/non_mr_table/' -statement error cannot restore individual table +statement ok +DROP TABLE non_mr_table + +statement ok +RESTORE TABLE non_mr_table FROM 'nodelocal://1/non_mr_table/' + +query TT +SHOW ZONE CONFIGURATION FROM TABLE non_mr_table +---- +RANGE default ALTER RANGE default CONFIGURE ZONE USING + range_min_bytes = 134217728, + range_max_bytes = 536870912, + gc.ttlseconds = 90000, + num_replicas = 3, + constraints = '[]', + lease_preferences = '[]' + +statement ok RESTORE TABLE non_mr_table FROM 'nodelocal://1/non_mr_table/' WITH into_db = 'mr-backup-1' + +statement ok +USE 'mr-backup-1' + +# Verify that the zone configurations of non_mr_table that is restored to a multi-region database are set. +query TT +SHOW ZONE CONFIGURATION FROM TABLE non_mr_table +---- +DATABASE "mr-backup-1" ALTER DATABASE "mr-backup-1" 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]]' + +query TT +SHOW CREATE TABLE non_mr_table +---- +non_mr_table CREATE TABLE public.non_mr_table ( + i INT8 NULL, + rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), + CONSTRAINT "primary" PRIMARY KEY (rowid ASC), + FAMILY "primary" (i, rowid) + ) LOCALITY REGIONAL BY TABLE IN PRIMARY REGION + + +statement ok +USE "mr-backup-2" + +statement ok +DROP TABLE regional_by_row_table; +DROP TABLE regional_by_table_in_primary_region; +DROP TABLE regional_by_table_in_ca_central_1; +DROP TABLE global_table; + +# Restore individual tables into a database with the same regions. +subtest restore_tables_into_database_with_same_regions + +statement error cannot restore REGIONAL BY ROW table regional_by_row_table .* individually into a multi-region database +RESTORE TABLE regional_by_row_table FROM 'nodelocal://1/mr-backup-2/' + +statement ok +RESTORE TABLE regional_by_table_in_primary_region FROM 'nodelocal://1/mr-backup-2/' + +query TT +SHOW CREATE TABLE regional_by_table_in_primary_region +---- +regional_by_table_in_primary_region CREATE TABLE public.regional_by_table_in_primary_region ( + pk INT8 NOT NULL, + i INT8 NULL, + CONSTRAINT "primary" PRIMARY KEY (pk ASC), + FAMILY fam_0_pk_i (pk, i) + ) LOCALITY REGIONAL BY TABLE IN PRIMARY REGION + + +statement ok +RESTORE TABLE regional_by_table_in_ca_central_1 FROM 'nodelocal://1/mr-backup-2/' + +# REGIONAL BY TABLE tables with a specific region are permitted if that region +# exists in the database. +query TT +SHOW CREATE TABLE regional_by_table_in_ca_central_1 +---- +regional_by_table_in_ca_central_1 CREATE TABLE public.regional_by_table_in_ca_central_1 ( + pk INT8 NOT NULL, + i INT8 NULL, + CONSTRAINT "primary" PRIMARY KEY (pk ASC), + FAMILY fam_0_pk_i (pk, i) + ) LOCALITY REGIONAL BY TABLE IN "ca-central-1" + +statement ok +RESTORE TABLE global_table FROM 'nodelocal://1/mr-backup-2/' + +query TT +SHOW CREATE TABLE global_table +---- +global_table CREATE TABLE public.global_table ( + pk INT8 NOT NULL, + i INT8 NULL, + CONSTRAINT "primary" PRIMARY KEY (pk ASC), + FAMILY fam_0_pk_i (pk, i) + ) LOCALITY GLOBAL + +# Restore individual tables into a database with different regions. +subtest restore_tables_into_database_with_different_regions + +# Create a database with different regions (missing ca-central-1). +statement ok +CREATE DATABASE "mr-restore-1" primary region "ap-southeast-2" regions "us-east-1" + +statement ok +RESTORE TABLE "mr-backup-2".global_table FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1'; +USE "mr-restore-1"; + +query TT +SHOW CREATE TABLE global_table +---- +global_table CREATE TABLE public.global_table ( + pk INT8 NOT NULL, + i INT8 NULL, + CONSTRAINT "primary" PRIMARY KEY (pk ASC), + FAMILY fam_0_pk_i (pk, i) + ) LOCALITY GLOBAL + +query TT +SHOW ZONE CONFIGURATION FROM TABLE global_table +---- +TABLE global_table ALTER TABLE global_table CONFIGURE ZONE USING + range_min_bytes = 134217728, + range_max_bytes = 536870912, + gc.ttlseconds = 90000, + global_reads = true, + num_replicas = 4, + num_voters = 3, + constraints = '{+region=ap-southeast-2: 1, +region=us-east-1: 1}', + voter_constraints = '[+region=ap-southeast-2]', + lease_preferences = '[[+region=ap-southeast-2]]' + + +statement ok +RESTORE TABLE "mr-backup-2".regional_by_table_in_primary_region FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1'; + +query TT +SHOW CREATE TABLE regional_by_table_in_primary_region +---- +regional_by_table_in_primary_region CREATE TABLE public.regional_by_table_in_primary_region ( + pk INT8 NOT NULL, + i INT8 NULL, + CONSTRAINT "primary" PRIMARY KEY (pk ASC), + FAMILY fam_0_pk_i (pk, i) + ) LOCALITY REGIONAL BY TABLE IN PRIMARY REGION + +query TT +SHOW ZONE CONFIGURATION FROM TABLE regional_by_table_in_primary_region +---- +DATABASE "mr-restore-1" ALTER DATABASE "mr-restore-1" 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=us-east-1: 1}', + voter_constraints = '[+region=ap-southeast-2]', + lease_preferences = '[[+region=ap-southeast-2]]' + +statement error "crdb_internal_region" is not compatible with type "crdb_internal_region" existing in cluster: could not find enum value "ca-central-1" in "crdb_internal_region" +RESTORE TABLE "mr-backup-2".regional_by_table_in_ap_southeast_2 FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1'; + +# Cannot restore a REGIONAL BY TABLE table that has different regions. +statement error cannot restore REGIONAL BY TABLE regional_by_table_in_ca_central_1 IN REGION "ca-central-1" \(table ID: [0-9]+\) into database "mr-restore-1"; region "ca-central-1" not found in database regions "ap-southeast-2", "us-east-1" +RESTORE TABLE "mr-backup-2".regional_by_table_in_ca_central_1 FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1' + +statement error cannot restore REGIONAL BY ROW table regional_by_row_table +RESTORE TABLE "mr-backup-2".regional_by_row_table FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1'