From c0b944fcd7e3ff52b9474aa7e5504dcf680b6a07 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Thu, 17 Mar 2022 15:07:19 -0400 Subject: [PATCH 1/2] sql: introduce zone config extension descriptor changes This commit introduces lightweight zone configuration extensions, which provide flexibility to multi-region abstractions and are loosely based on the design described in https://docs.google.com/document/d/1EiAT1BUOTaXoJy3CMzrKiP73bMfOAFiPrOPne5dtadU/edit#. ``` ALTER DATABASE db ALTER LOCALITY REGIONAL CONFIGURE ZONE USING ... ALTER DATABASE db ALTER LOCALITY REGIONAL IN "us-east1" CONFIGURE ZONE USING ... ALTER DATABASE db ALTER LOCALITY GLOBAL CONFIGURE ZONE USING ... ``` Zone config extensions represents per-locality zone configurations that influence the zone configurations derived for corresponding objects. A locality type's associated extension acts as a targeted set of rewrite rules for its associated objects' (database, table, partition) derived zone configurations. "Extending" a zone config means having the extension inherit any missing fields from the zone config while replacing any set fields. This uses the existing zone configuration inheritance rules. Rules (implemented in `pkg/sql/region_util.go`): 1. database-level zone configs are extended first using the regional zone config extension and then with the regional_in[] zone config extension. In doing so, REGIONAL [ IN PRIMARY REGION ] tables can continue to inherit from the database level zone config. 2. table-level zone configs use the extension associated with their locality type. 2a. GLOBAL tables are extended with the global zone config extension. 2b. REGIONAL IN tables are extended first using the regional zone config extension and then with the regional_in[] zone config extension. 3. partition-level zone configs are extended first using the regional zone config extension and then with the regional_in[] zone config extension. The commit contains only the descriptor changes and the logic to interpret zone config extensions during multi-region zone config synthesis. It does not contain the logic to set the extensions. As a result, it may be contained enough to fit into the v22.1.0 release, allowing rest of the code to land in a later point release. --- pkg/ccl/backupccl/restore_job.go | 7 +- pkg/ccl/backupccl/restore_planning.go | 1 + pkg/ccl/multiregionccl/BUILD.bazel | 1 + pkg/ccl/multiregionccl/multiregion.go | 9 +- .../testdata/end_to_end/drop_multiregion | 8 +- pkg/sql/catalog/descpb/BUILD.bazel | 2 + pkg/sql/catalog/descpb/structured.proto | 43 + pkg/sql/catalog/descriptor.go | 3 + pkg/sql/catalog/multiregion/BUILD.bazel | 1 + pkg/sql/catalog/multiregion/region_config.go | 118 +- .../catalog/multiregion/region_config_test.go | 246 ++- .../typedesc/table_implicit_record_type.go | 6 + pkg/sql/catalog/typedesc/type_desc.go | 22 +- pkg/sql/region_util.go | 349 ++-- pkg/sql/region_util_test.go | 1819 ++++++++++++----- 15 files changed, 1933 insertions(+), 702 deletions(-) diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go index 039f55b342c4..82a4a1090f8a 100644 --- a/pkg/ccl/backupccl/restore_job.go +++ b/pkg/ccl/backupccl/restore_job.go @@ -875,7 +875,11 @@ func createImportingDescriptors( if err != nil { return err } - superRegions, err := t.SuperRegions() + superRegions, err := typeDesc.SuperRegions() + if err != nil { + return err + } + zoneCfgExtensions, err := typeDesc.ZoneConfigExtensions() if err != nil { return err } @@ -886,6 +890,7 @@ func createImportingDescriptors( desc.RegionConfig.RegionEnumID, desc.RegionConfig.Placement, superRegions, + zoneCfgExtensions, ) if err := sql.ApplyZoneConfigFromDatabaseRegionConfig( ctx, diff --git a/pkg/ccl/backupccl/restore_planning.go b/pkg/ccl/backupccl/restore_planning.go index 89441eac2680..9566b8709753 100644 --- a/pkg/ccl/backupccl/restore_planning.go +++ b/pkg/ccl/backupccl/restore_planning.go @@ -2054,6 +2054,7 @@ func planDatabaseModifiersForRestore( regionEnumID, descpb.DataPlacement_DEFAULT, nil, + descpb.ZoneConfigExtensions{}, ) if err := multiregion.ValidateRegionConfig(regionConfig); err != nil { return nil, nil, err diff --git a/pkg/ccl/multiregionccl/BUILD.bazel b/pkg/ccl/multiregionccl/BUILD.bazel index 9c6cedb3979b..7996724f5953 100644 --- a/pkg/ccl/multiregionccl/BUILD.bazel +++ b/pkg/ccl/multiregionccl/BUILD.bazel @@ -10,6 +10,7 @@ go_library( "//pkg/sql", "//pkg/sql/catalog/catpb", "//pkg/sql/catalog/descidgen", + "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/multiregion", "//pkg/sql/catalog/typedesc", "//pkg/sql/pgwire/pgcode", diff --git a/pkg/ccl/multiregionccl/multiregion.go b/pkg/ccl/multiregionccl/multiregion.go index bcd6d2f4b6f8..a85ad8e0a4c0 100644 --- a/pkg/ccl/multiregionccl/multiregion.go +++ b/pkg/ccl/multiregionccl/multiregion.go @@ -16,6 +16,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descidgen" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/multiregion" "github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -97,7 +98,13 @@ func initializeMultiRegionMetadata( return nil, err } regionConfig := multiregion.MakeRegionConfig( - regionNames, primaryRegion, survivalGoal, regionEnumID, placement, nil, + regionNames, + primaryRegion, + survivalGoal, + regionEnumID, + placement, + nil, + descpb.ZoneConfigExtensions{}, ) if err := multiregion.ValidateRegionConfig(regionConfig); err != nil { return nil, err diff --git a/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_multiregion b/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_multiregion index 5c67ded00c47..1ca05b5ec2ff 100644 --- a/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_multiregion +++ b/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_multiregion @@ -39,6 +39,7 @@ upsert descriptor #105 - - 108 regionConfig: primaryRegion: us-east1 + zoneConfigExtensions: {} - version: "2" + version: "3" upsert descriptor #107 @@ -375,8 +376,8 @@ upsert descriptor #105 enumMembers: - logicalRepresentation: us-east1 ... - regionConfig: primaryRegion: us-east1 + zoneConfigExtensions: {} - version: "3" + version: "4" upsert descriptor #107 @@ -731,6 +732,7 @@ upsert descriptor #105 - - 109 regionConfig: primaryRegion: us-east1 + zoneConfigExtensions: {} - version: "5" + version: "6" upsert descriptor #109 @@ -959,8 +961,8 @@ upsert descriptor #105 enumMembers: - logicalRepresentation: us-east1 ... - regionConfig: primaryRegion: us-east1 + zoneConfigExtensions: {} - version: "6" + version: "7" upsert descriptor #109 @@ -1395,8 +1397,8 @@ upsert descriptor #105 enumMembers: - logicalRepresentation: us-east1 ... - regionConfig: primaryRegion: us-east1 + zoneConfigExtensions: {} - version: "7" + state: DROP + version: "8" diff --git a/pkg/sql/catalog/descpb/BUILD.bazel b/pkg/sql/catalog/descpb/BUILD.bazel index 3b051ca0232b..4d5d09080d09 100644 --- a/pkg/sql/catalog/descpb/BUILD.bazel +++ b/pkg/sql/catalog/descpb/BUILD.bazel @@ -59,6 +59,7 @@ proto_library( strip_import_prefix = "/pkg", visibility = ["//visibility:public"], deps = [ + "//pkg/config/zonepb:zonepb_proto", "//pkg/geo/geoindex:geoindex_proto", "//pkg/roachpb:roachpb_proto", "//pkg/sql/catalog/catpb:catpb_proto", @@ -76,6 +77,7 @@ go_proto_library( proto = ":descpb_proto", visibility = ["//visibility:public"], deps = [ + "//pkg/config/zonepb", "//pkg/geo/geoindex", "//pkg/roachpb", # keep "//pkg/sql/catalog/catpb", diff --git a/pkg/sql/catalog/descpb/structured.proto b/pkg/sql/catalog/descpb/structured.proto index 4306d5a8b6be..258beadae90b 100644 --- a/pkg/sql/catalog/descpb/structured.proto +++ b/pkg/sql/catalog/descpb/structured.proto @@ -13,6 +13,7 @@ syntax = "proto2"; package cockroach.sql.sqlbase; option go_package = "descpb"; +import "config/zonepb/zone.proto"; import "util/hlc/timestamp.proto"; import "sql/catalog/catpb/catalog.proto"; import "sql/catalog/catpb/privilege.proto"; @@ -1306,6 +1307,44 @@ message SuperRegion { repeated string regions = 2 [(gogoproto.casttype)="github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.RegionName"]; } +// ZoneConfigExtensions represents per-locality zone configurations that +// influence the zone configurations derived for corresponding objects. A +// locality type's associated extension acts as a targeted set of rewrite rules +// for its associated objects' (database, table, partition) derived zone +// configurations. +// +// "extending" a zone config means having the extension inherit any missing +// fields from the zone config while replacing any set fields. This uses the +// existing zone configuration inheritance rules. +// +// Rules (implemented in pkg/sql/region_util.go): +// 1. database-level zone configs are extended first using the regional zone +// config extension and then with the regional_in[] zone +// config extension. In doing so, REGIONAL [ IN PRIMARY REGION ] tables can +// continue to inherit from the database level zone config. +// 2. table-level zone configs use the extension associated with their locality +// type. +// 2a. GLOBAL tables are extended with the global zone config extension. +// 2b. REGIONAL IN tables are extended first using the regional zone +// config extension and then with the regional_in[] zone +// config extension. +// 3. partition-level zone configs are extended first using the regional zone +// config extension and then with the regional_in[] zone +// config extension. +message ZoneConfigExtensions { + option (gogoproto.equal) = true; + + // Global extends the zone config applied to GLOBAL tables. + optional config.zonepb.ZoneConfig global = 1; + // Regional extends the zone config applied to REGIONAL [ BY ROW ] tables and + // partitions, regardless of affinity region. + optional config.zonepb.ZoneConfig regional = 2; + // RegionalIn further extends the zone configs applied to REGIONAL [ BY ROW ] + // tables and partitions, according to affinity region. + map regional_in = 3 [(gogoproto.nullable) = false, + (gogoproto.castkey)="github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.RegionName"]; +} + // TypeDescriptor represents a user defined type and is stored in a structured // metadata key. The TypeDescriptor has a globally-unique ID shared with other // Descriptors. @@ -1425,6 +1464,10 @@ message TypeDescriptor { // SuperRegions represents the super regions defined on the database. repeated SuperRegion super_regions = 2 [(gogoproto.nullable) = false]; + + // ZoneConfigExtensions represents per-locality zone configurations that + // influence the zone configurations derived for corresponding objects. + optional ZoneConfigExtensions zone_config_extensions = 3 [(gogoproto.nullable) = false]; } optional RegionConfig region_config = 16; diff --git a/pkg/sql/catalog/descriptor.go b/pkg/sql/catalog/descriptor.go index e56e2b07d974..c11099f853ea 100644 --- a/pkg/sql/catalog/descriptor.go +++ b/pkg/sql/catalog/descriptor.go @@ -738,6 +738,9 @@ type TypeDescriptor interface { TransitioningRegionNames() (catpb.RegionNames, error) // SuperRegions returns the list of super regions. SuperRegions() ([]descpb.SuperRegion, error) + // ZoneConfigExtensions returns the zone configuration extensions on the + // multi-region enum. + ZoneConfigExtensions() (descpb.ZoneConfigExtensions, error) // The following fields are set if the type is an enum or a multi-region enum. diff --git a/pkg/sql/catalog/multiregion/BUILD.bazel b/pkg/sql/catalog/multiregion/BUILD.bazel index d75147a647ca..08f5829ceb0b 100644 --- a/pkg/sql/catalog/multiregion/BUILD.bazel +++ b/pkg/sql/catalog/multiregion/BUILD.bazel @@ -9,6 +9,7 @@ go_library( importpath = "github.com/cockroachdb/cockroach/pkg/sql/catalog/multiregion", visibility = ["//visibility:public"], deps = [ + "//pkg/config/zonepb", "//pkg/sql/catalog", "//pkg/sql/catalog/catpb", "//pkg/sql/catalog/descpb", diff --git a/pkg/sql/catalog/multiregion/region_config.go b/pkg/sql/catalog/multiregion/region_config.go index 7f9375055b44..6ffd2391e0bf 100644 --- a/pkg/sql/catalog/multiregion/region_config.go +++ b/pkg/sql/catalog/multiregion/region_config.go @@ -15,6 +15,7 @@ package multiregion import ( "sort" + "github.com/cockroachdb/cockroach/pkg/config/zonepb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -39,6 +40,7 @@ type RegionConfig struct { regionEnumID descpb.ID placement descpb.DataPlacement superRegions []descpb.SuperRegion + zoneCfgExtensions descpb.ZoneConfigExtensions } // SurvivalGoal returns the survival goal configured on the RegionConfig. @@ -56,6 +58,14 @@ func (r *RegionConfig) Regions() catpb.RegionNames { return r.regions } +// WithRegions returns a copy of the RegionConfig with only the provided +// regions. +func (r *RegionConfig) WithRegions(regions catpb.RegionNames) RegionConfig { + cpy := *r + cpy.regions = regions + return cpy +} + // IsMemberOfExplicitSuperRegion returns whether t the region is an explicit // member of a super region. func (r *RegionConfig) IsMemberOfExplicitSuperRegion(region catpb.RegionName) bool { @@ -66,24 +76,24 @@ func (r *RegionConfig) IsMemberOfExplicitSuperRegion(region catpb.RegionName) bo } } } - return false } // GetSuperRegionRegionsForRegion returns the members of the super region the // specified region is part of. -// If the region is not a member of any super regions, we return all -// the regions on the RegionConfig. -func (r *RegionConfig) GetSuperRegionRegionsForRegion(region catpb.RegionName) catpb.RegionNames { +// If the region is not a member of any super regions, the function returns an +// error. +func (r *RegionConfig) GetSuperRegionRegionsForRegion( + region catpb.RegionName, +) (catpb.RegionNames, error) { for _, superRegion := range r.SuperRegions() { for _, regionOfSuperRegion := range superRegion.Regions { if region == regionOfSuperRegion { - return superRegion.Regions + return superRegion.Regions, nil } } } - - return r.Regions() + return nil, errors.AssertionFailedf("region %s is not part of a super region", region) } // IsValidRegionNameString implements the tree.DatabaseRegionConfig interface. @@ -123,11 +133,91 @@ func (r *RegionConfig) IsPlacementRestricted() bool { return r.placement == descpb.DataPlacement_RESTRICTED } +// WithPlacementDefault returns a copy of the RegionConfig with the data +// placement strategy configured as placement default. +func (r *RegionConfig) WithPlacementDefault() RegionConfig { + cpy := *r + cpy.placement = descpb.DataPlacement_DEFAULT + return cpy +} + // SuperRegions returns the list of super regions in the database. func (r *RegionConfig) SuperRegions() []descpb.SuperRegion { return r.superRegions } +// ApplyZoneConfigExtensionForGlobal applies the global table zone configuration +// extensions to the provided zone configuration, returning the updated config. +func (r *RegionConfig) ApplyZoneConfigExtensionForGlobal(zc zonepb.ZoneConfig) zonepb.ZoneConfig { + if ext := r.zoneCfgExtensions.Global; ext != nil { + zc = extendZoneCfg(zc, *ext) + } + return zc +} + +// ApplyZoneConfigExtensionForRegionalIn applies the regional table zone +// configuration extensions for the provided region to the provided zone +// configuration, returning the updated config. +func (r *RegionConfig) ApplyZoneConfigExtensionForRegionalIn( + zc zonepb.ZoneConfig, region catpb.RegionName, +) zonepb.ZoneConfig { + if ext := r.zoneCfgExtensions.Regional; ext != nil { + zc = extendZoneCfg(zc, *ext) + } + if ext, ok := r.zoneCfgExtensions.RegionalIn[region]; ok { + zc = extendZoneCfg(zc, ext) + } + return zc +} + +// "extending" a zone config means having the extension inherit any missing +// fields from the zone config while replacing any set fields. +func extendZoneCfg(zc, ext zonepb.ZoneConfig) zonepb.ZoneConfig { + ext.InheritFromParent(&zc) + return ext +} + +// GlobalTablesInheritDatabaseConstraints returns whether GLOBAL tables can +// inherit replica constraints from their database zone configuration, or +// whether they must set these constraints themselves. +func (r *RegionConfig) GlobalTablesInheritDatabaseConstraints() bool { + if r.placement == descpb.DataPlacement_RESTRICTED { + // Placement restricted does not apply to GLOBAL tables. + return false + } + if r.zoneCfgExtensions.Global != nil { + // Global tables have a zone config extension that will not be set at + // the database level. + return false + } + if r.zoneCfgExtensions.Regional != nil { + // Regional tables have a zone config extension that will be set at the + // database level but which should not apply to GLOBAL tables. + return false + } + if _, ok := r.zoneCfgExtensions.RegionalIn[r.primaryRegion]; ok { + // Regional tables in the primary region have a zone config extension that + // will be set at the database level but which should not apply to GLOBAL + // tables. + return false + } + return true +} + +// RegionalInTablesInheritDatabaseConstraints returns whether REGIONAL +// tables/partitions with affinity to the specified region can inherit replica +// constraints from their database zone configuration, or whether they must set +// these constraints themselves. +func (r *RegionConfig) RegionalInTablesInheritDatabaseConstraints(region catpb.RegionName) bool { + if _, ok := r.zoneCfgExtensions.RegionalIn[r.primaryRegion]; ok { + // Regional tables in the primary region have a zone config extension that + // will be set at the database level but which should not apply to regional + // tables in any other region. + return r.primaryRegion == region + } + return true +} + // MakeRegionConfigOption is an option for MakeRegionConfig type MakeRegionConfigOption func(r *RegionConfig) @@ -147,15 +237,17 @@ func MakeRegionConfig( regionEnumID descpb.ID, placement descpb.DataPlacement, superRegions []descpb.SuperRegion, + zoneCfgExtensions descpb.ZoneConfigExtensions, opts ...MakeRegionConfigOption, ) RegionConfig { ret := RegionConfig{ - regions: regions, - primaryRegion: primaryRegion, - survivalGoal: survivalGoal, - regionEnumID: regionEnumID, - placement: placement, - superRegions: superRegions, + regions: regions, + primaryRegion: primaryRegion, + survivalGoal: survivalGoal, + regionEnumID: regionEnumID, + placement: placement, + superRegions: superRegions, + zoneCfgExtensions: zoneCfgExtensions, } for _, opt := range opts { opt(&ret) diff --git a/pkg/sql/catalog/multiregion/region_config_test.go b/pkg/sql/catalog/multiregion/region_config_test.go index 8474e4c5d215..6987b193c0d9 100644 --- a/pkg/sql/catalog/multiregion/region_config_test.go +++ b/pkg/sql/catalog/multiregion/region_config_test.go @@ -32,25 +32,57 @@ func TestValidateRegionConfig(t *testing.T) { }{ { err: "expected a valid multi-region enum ID", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_a", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + }, "region_b", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), }, { err: "3 regions are required for surviving a region failure", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_a", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + }, "region_b", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_REGION_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), }, { - err: "expected > 0 number of regions in the region config", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{}, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, nil), + err: "expected > 0 number of regions in the region config", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{}, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), }, { - err: "cannot have a database with restricted placement that is also region survivable", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validRegionEnumID, descpb.DataPlacement_RESTRICTED, nil), + err: "cannot have a database with restricted placement that is also region survivable", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{"region_a", "region_b", "region_c"}, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + validRegionEnumID, + descpb.DataPlacement_RESTRICTED, + nil, + descpb.ZoneConfigExtensions{}, + ), }, } @@ -81,94 +113,190 @@ func TestValidateSuperRegionConfig(t *testing.T) { { testName: "region names within a super region should be sorted", err: "the regions within super region sr1 were not in a sorted order", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{"region_b", "region_a"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", }, - }), + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_b", "region_a"}, + }, + }, + descpb.ZoneConfigExtensions{}, + ), }, { testName: "regions should be unique within a super region", err: "duplicate region region_b found in super region sr1", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{"region_b", "region_b"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_b", "region_b"}, + }, }, - }), + descpb.ZoneConfigExtensions{}, + ), }, { testName: "regions within a super region should map to a valid region on the database", err: "region region_d not part of database", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{"region_d"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", }, - }), + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_d"}, + }, + }, + descpb.ZoneConfigExtensions{}, + ), }, { testName: "super region names should be sorted", err: "super regions are not in sorted order based on the super region name", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "sr2", - Regions: []catpb.RegionName{"region_a"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", }, - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{"region_b"}, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr2", + Regions: []catpb.RegionName{"region_a"}, + }, + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_b"}, + }, }, - }), + descpb.ZoneConfigExtensions{}, + ), }, { testName: "a region can only appear in one super region", err: "region region_a found in multiple super regions", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{"region_a"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", }, - { - SuperRegionName: "sr2", - Regions: []catpb.RegionName{"region_a"}, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_a"}, + }, + { + SuperRegionName: "sr2", + Regions: []catpb.RegionName{"region_a"}, + }, }, - }), + descpb.ZoneConfigExtensions{}, + ), }, { testName: "super region names must be unique", err: "duplicate super regions with name sr1 found", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{"region_a"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", }, - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{"region_a"}, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_a"}, + }, + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_a"}, + }, }, - }), + descpb.ZoneConfigExtensions{}, + ), }, { testName: "a super region should have at least one region", err: "no regions found within super region sr1", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", }, - }), + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{}, + }, + }, + descpb.ZoneConfigExtensions{}, + ), }, { testName: "a super region should have at least three regions if the survival mode is region failure", err: "super region sr1 only has 2 region(s): at least 3 regions are required for surviving a region failure", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{"region_a", "region_b", "region_c"}, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "sr1", - Regions: []catpb.RegionName{"region_a", "region_b"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_a", "region_b"}, + }, }, - }), + descpb.ZoneConfigExtensions{}, + ), }, } diff --git a/pkg/sql/catalog/typedesc/table_implicit_record_type.go b/pkg/sql/catalog/typedesc/table_implicit_record_type.go index 2b3b6186f778..ec506697b6da 100644 --- a/pkg/sql/catalog/typedesc/table_implicit_record_type.go +++ b/pkg/sql/catalog/typedesc/table_implicit_record_type.go @@ -307,6 +307,12 @@ func (v TableImplicitRecordType) SuperRegions() ([]descpb.SuperRegion, error) { ) } +// ZoneConfigExtensions implements the TypeDescriptorInterface. +func (v TableImplicitRecordType) ZoneConfigExtensions() (descpb.ZoneConfigExtensions, error) { + return descpb.ZoneConfigExtensions{}, errors.AssertionFailedf( + "can not get the zone config extensions of a implicit table record type") +} + // GetArrayTypeID implements the TypeDescriptorInterface. func (v TableImplicitRecordType) GetArrayTypeID() descpb.ID { return 0 diff --git a/pkg/sql/catalog/typedesc/type_desc.go b/pkg/sql/catalog/typedesc/type_desc.go index 94b710c0fc98..9387636c8514 100644 --- a/pkg/sql/catalog/typedesc/type_desc.go +++ b/pkg/sql/catalog/typedesc/type_desc.go @@ -202,7 +202,7 @@ func (desc *immutable) NewBuilder() catalog.DescriptorBuilder { func (desc *immutable) PrimaryRegionName() (catpb.RegionName, error) { if desc.Kind != descpb.TypeDescriptor_MULTIREGION_ENUM { return "", errors.AssertionFailedf( - "can not get primary region of a non multi-region enum") + "can not get primary region of a non multi-region enum %q (%d)", desc.GetName(), desc.GetID()) } return desc.RegionConfig.PrimaryRegion, nil } @@ -211,7 +211,7 @@ func (desc *immutable) PrimaryRegionName() (catpb.RegionName, error) { func (desc *immutable) RegionNames() (catpb.RegionNames, error) { if desc.Kind != descpb.TypeDescriptor_MULTIREGION_ENUM { return nil, errors.AssertionFailedf( - "can not get regions of a non multi-region enum %d", desc.ID, + "can not get regions of a non multi-region enum %q (%d)", desc.GetName(), desc.GetID(), ) } var regions catpb.RegionNames @@ -228,7 +228,7 @@ func (desc *immutable) RegionNames() (catpb.RegionNames, error) { func (desc *immutable) TransitioningRegionNames() (catpb.RegionNames, error) { if desc.Kind != descpb.TypeDescriptor_MULTIREGION_ENUM { return nil, errors.AssertionFailedf( - "can not get regions of a non multi-region enum %d", desc.ID, + "can not get regions of a non multi-region enum %q (%d)", desc.GetName(), desc.GetID(), ) } var regions catpb.RegionNames @@ -244,7 +244,7 @@ func (desc *immutable) TransitioningRegionNames() (catpb.RegionNames, error) { func (desc *immutable) RegionNamesForValidation() (catpb.RegionNames, error) { if desc.Kind != descpb.TypeDescriptor_MULTIREGION_ENUM { return nil, errors.AssertionFailedf( - "can not get regions of a non multi-region enum %d", desc.ID, + "can not get regions of a non multi-region enum %q (%d)", desc.GetName(), desc.GetID(), ) } var regions catpb.RegionNames @@ -262,7 +262,7 @@ func (desc *immutable) RegionNamesForValidation() (catpb.RegionNames, error) { func (desc *immutable) SuperRegions() ([]descpb.SuperRegion, error) { if desc.Kind != descpb.TypeDescriptor_MULTIREGION_ENUM { return nil, errors.AssertionFailedf( - "can not get regions of a non multi-region enum %d", desc.ID, + "can not get regions of a non multi-region enum %q (%d)", desc.GetName(), desc.GetID(), ) } @@ -273,7 +273,7 @@ func (desc *immutable) SuperRegions() ([]descpb.SuperRegion, error) { func (desc *immutable) RegionNamesIncludingTransitioning() (catpb.RegionNames, error) { if desc.Kind != descpb.TypeDescriptor_MULTIREGION_ENUM { return nil, errors.AssertionFailedf( - "can not get regions of a non multi-region enum %d", desc.ID, + "can not get regions of a non multi-region enum %q (%d)", desc.GetName(), desc.GetID(), ) } var regions catpb.RegionNames @@ -283,6 +283,16 @@ func (desc *immutable) RegionNamesIncludingTransitioning() (catpb.RegionNames, e return regions, nil } +// ZoneConfigExtensions implements the TypeDescriptorInterface. +func (desc *immutable) ZoneConfigExtensions() (descpb.ZoneConfigExtensions, error) { + if desc.Kind != descpb.TypeDescriptor_MULTIREGION_ENUM { + return descpb.ZoneConfigExtensions{}, errors.AssertionFailedf( + "can not get the zone config extensions of a non multi-region enum %q (%d)", desc.GetName(), desc.GetID(), + ) + } + return desc.RegionConfig.ZoneConfigExtensions, nil +} + // SetDrainingNames implements the MutableDescriptor interface. // // Deprecated: Do not use. diff --git a/pkg/sql/region_util.go b/pkg/sql/region_util.go index 211661860a9a..3ff17db3bea3 100644 --- a/pkg/sql/region_util.go +++ b/pkg/sql/region_util.go @@ -141,30 +141,11 @@ func makeRequiredConstraintForRegion(r catpb.RegionName) zonepb.Constraint { func zoneConfigForMultiRegionDatabase( regionConfig multiregion.RegionConfig, ) (zonepb.ZoneConfig, error) { - numVoters, numReplicas := getNumVotersAndNumReplicasForDefaultDatabaseRegions(regionConfig) - var constraints []zonepb.ConstraintsConjunction - if regionConfig.IsPlacementRestricted() { - // In a RESTRICTED placement policy, the database zone config has no - // non-voters so that REGIONAL BY [TABLE | ROW] can inherit the RESTRICTED - // placement. Voter placement will be set at the table/partition level to - // the table/partition region. + numVoters, numReplicas := getNumVotersAndNumReplicas(regionConfig) - // NB: When setting empty constraints, use nil as opposed to []. When - // constraints are deserialized from the database, empty constraints are - // always deserialized as nil. Therefore, if constraints are set as [] here, - // the database will have a difference in its expected constraints vs the - // actual constraints when comparing using the multi-region validation - // builtins. - constraints = nil - } else { - constraints = make([]zonepb.ConstraintsConjunction, len(regionConfig.Regions())) - for i, region := range regionConfig.Regions() { - // Constrain at least 1 (voting or non-voting) replica per region. - constraints[i] = zonepb.ConstraintsConjunction{ - NumReplicas: 1, - Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(region)}, - } - } + constraints, err := synthesizeReplicaConstraints(regionConfig.Regions(), regionConfig.Placement()) + if err != nil { + return zonepb.ZoneConfig{}, err } voterConstraints, err := synthesizeVoterConstraints(regionConfig.PrimaryRegion(), regionConfig) @@ -172,41 +153,42 @@ func zoneConfigForMultiRegionDatabase( return zonepb.ZoneConfig{}, err } - return zonepb.ZoneConfig{ - NumReplicas: &numReplicas, - NumVoters: &numVoters, - LeasePreferences: []zonepb.LeasePreference{ - {Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(regionConfig.PrimaryRegion())}}, - }, - NullVoterConstraintsIsEmpty: true, - VoterConstraints: voterConstraints, + leasePreferences := synthesizeLeasePreferences(regionConfig.PrimaryRegion()) + + zc := zonepb.ZoneConfig{ + NumReplicas: &numReplicas, + NumVoters: &numVoters, Constraints: constraints, - }, nil -} - -// maybeAddConstraintsForSuperRegion updates the ZoneConfig.Constraints field -// such that every replica is guaranteed to be constrained to a region -// within the super region. -// If primaryRegion is not a member of any super region, there is nothing -// to be done. -func maybeAddConstraintsForSuperRegion( - primaryRegion catpb.RegionName, - regions catpb.RegionNames, - zc *zonepb.ZoneConfig, - numReplicas int32, - regionConfig multiregion.RegionConfig, -) { - if !regionConfig.IsMemberOfExplicitSuperRegion(primaryRegion) { - return + VoterConstraints: voterConstraints, + LeasePreferences: leasePreferences, + InheritedConstraints: false, + NullVoterConstraintsIsEmpty: true, + InheritedLeasePreferences: false, + } + + zc = regionConfig.ApplyZoneConfigExtensionForRegionalIn(zc, regionConfig.PrimaryRegion()) + return zc, nil +} + +// addConstraintsForSuperRegion updates the ZoneConfig.Constraints field such +// that every replica is guaranteed to be constrained to a region within the +// super region. +// If !regionConfig.IsMemberOfExplicitSuperRegion(affinityRegion), and error +// will be returned. +func addConstraintsForSuperRegion( + zc *zonepb.ZoneConfig, regionConfig multiregion.RegionConfig, affinityRegion catpb.RegionName, +) error { + regions, err := regionConfig.GetSuperRegionRegionsForRegion(affinityRegion) + if err != nil { + return err } + _, numReplicas := getNumVotersAndNumReplicas(regionConfig.WithRegions(regions)) zc.NumReplicas = &numReplicas zc.Constraints = nil zc.InheritedConstraints = false - zc.InheritedLeasePreferences = false - survivalGoal := regionConfig.SurvivalGoal() - switch survivalGoal { + switch regionConfig.SurvivalGoal() { case descpb.SurvivalGoal_ZONE_FAILURE: for _, region := range regions { zc.Constraints = append(zc.Constraints, zonepb.ConstraintsConjunction{ @@ -214,6 +196,7 @@ func maybeAddConstraintsForSuperRegion( Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(region)}, }) } + return nil case descpb.SurvivalGoal_REGION_FAILURE: // There is a special case where we have 3 regions under survival goal // region failure where we have to constrain an extra replica to any @@ -230,7 +213,7 @@ func maybeAddConstraintsForSuperRegion( extraReplicaToConstrain := len(regions) == 3 for _, region := range regions { n := int32(1) - if region != primaryRegion && extraReplicaToConstrain { + if region != affinityRegion && extraReplicaToConstrain { n = 2 extraReplicaToConstrain = false } @@ -239,8 +222,9 @@ func maybeAddConstraintsForSuperRegion( Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(region)}, }) } + return nil default: - panic(fmt.Sprintf("unknown survival goal %s", survivalGoal)) + return errors.AssertionFailedf("unknown survival goal: %v", regionConfig.SurvivalGoal()) } } @@ -254,30 +238,42 @@ func maybeAddConstraintsForSuperRegion( func zoneConfigForMultiRegionPartition( partitionRegion catpb.RegionName, regionConfig multiregion.RegionConfig, ) (zonepb.ZoneConfig, error) { - zc := zonepb.NewZoneConfig() + zc := *zonepb.NewZoneConfig() + + numVoters, numReplicas := getNumVotersAndNumReplicas(regionConfig) + zc.NumVoters = &numVoters + + if regionConfig.IsMemberOfExplicitSuperRegion(partitionRegion) { + err := addConstraintsForSuperRegion(&zc, regionConfig, partitionRegion) + if err != nil { + return zonepb.ZoneConfig{}, err + } + } else if !regionConfig.RegionalInTablesInheritDatabaseConstraints(partitionRegion) { + // If the database constraints can't be inherited to serve as the + // constraints for this partition, define the constraints ourselves. + zc.NumReplicas = &numReplicas + + constraints, err := synthesizeReplicaConstraints(regionConfig.Regions(), regionConfig.Placement()) + if err != nil { + return zonepb.ZoneConfig{}, err + } + zc.Constraints = constraints + zc.InheritedConstraints = false + } + voterConstraints, err := synthesizeVoterConstraints(partitionRegion, regionConfig) if err != nil { return zonepb.ZoneConfig{}, err } - - zc.NullVoterConstraintsIsEmpty = true zc.VoterConstraints = voterConstraints + zc.NullVoterConstraintsIsEmpty = true + leasePreferences := synthesizeLeasePreferences(partitionRegion) + zc.LeasePreferences = leasePreferences zc.InheritedLeasePreferences = false - zc.LeasePreferences = []zonepb.LeasePreference{ - {Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(partitionRegion)}}, - } - - regions := regionConfig.GetSuperRegionRegionsForRegion(partitionRegion) - numVoters, numReplicas := getNumVotersAndNumReplicas( - len(regions), regionConfig.SurvivalGoal(), regionConfig.IsPlacementRestricted(), - ) - zc.NumVoters = &numVoters - - maybeAddConstraintsForSuperRegion(partitionRegion, regions, zc, numReplicas, regionConfig) - - return *zc, err + zc = regionConfig.ApplyZoneConfigExtensionForRegionalIn(zc, partitionRegion) + return zc, err } // maxFailuresBeforeUnavailability returns the maximum number of individual @@ -287,18 +283,10 @@ func maxFailuresBeforeUnavailability(numVoters int32) int32 { return ((numVoters + 1) / 2) - 1 } -// getNumVotersAndNumReplicasForDefaultDatabaseRegions computes the number of -// voters and the total number of replicas needed for a given region config. -func getNumVotersAndNumReplicasForDefaultDatabaseRegions( - config multiregion.RegionConfig, -) (numVoters, numReplicas int32) { - return getNumVotersAndNumReplicas( - len(config.Regions()), config.SurvivalGoal(), config.IsPlacementRestricted(), - ) -} - +// getNumVotersAndNumReplicas computes the number of voters and the total number +// of replicas needed for a given region config. func getNumVotersAndNumReplicas( - numRegions int, survivalGoal descpb.SurvivalGoal, isPlacementRestricted bool, + regionConfig multiregion.RegionConfig, ) (numVoters, numReplicas int32) { const numVotersForZoneSurvival = 3 // Under region survivability, we use 5 voting replicas to allow for a @@ -316,16 +304,20 @@ func getNumVotersAndNumReplicas( // may or may not be placed geographically close to the leaseholder replica. const numVotersForRegionSurvival = 5 - switch survivalGoal { + numRegions := int32(len(regionConfig.Regions())) + switch regionConfig.SurvivalGoal() { // NB: See mega-comment inside `synthesizeVoterConstraints()` for why these // are set the way they are. case descpb.SurvivalGoal_ZONE_FAILURE: numVoters = numVotersForZoneSurvival - if isPlacementRestricted { - numReplicas = numVoters - } else { + switch regionConfig.Placement() { + case descpb.DataPlacement_DEFAULT: // + <1 replica for every other region> - numReplicas = (numVotersForZoneSurvival) + (int32(numRegions) - 1) + numReplicas = (numVotersForZoneSurvival) + (numRegions - 1) + case descpb.DataPlacement_RESTRICTED: + numReplicas = numVoters + default: + panic(errors.AssertionFailedf("unknown data placement: %v", regionConfig.Placement())) } case descpb.SurvivalGoal_REGION_FAILURE: // <(quorum - 1) voters in the home region> + <1 replica for every other @@ -334,7 +326,7 @@ func getNumVotersAndNumReplicas( // We place the maximum concurrent replicas that can fail before a range // outage in the home region, and ensure that there's at least one replica // in all other regions. - numReplicas = maxFailuresBeforeUnavailability(numVotersForRegionSurvival) + (int32(numRegions) - 1) + numReplicas = maxFailuresBeforeUnavailability(numVotersForRegionSurvival) + (numRegions - 1) if numReplicas < numVoters { // NumReplicas cannot be less than NumVoters. If we have <= 4 regions, all // replicas will be voting replicas. @@ -346,7 +338,8 @@ func getNumVotersAndNumReplicas( // synthesizeVoterConstraints generates a ConstraintsConjunction clause // representing the `voter_constraints` field to be set for the primary region -// of a multi-region database or the home region of a table in such a database. +// of a multi-region database or the home region of a table/partition in such a +// database. // // Under zone survivability, we will constrain all voting replicas to be inside // the primary/home region. @@ -397,7 +390,7 @@ func synthesizeVoterConstraints( }, }, nil case descpb.SurvivalGoal_REGION_FAILURE: - numVoters, _ := getNumVotersAndNumReplicasForDefaultDatabaseRegions(regionConfig) + numVoters, _ := getNumVotersAndNumReplicas(regionConfig) return []zonepb.ConstraintsConjunction{ { // We constrain voting replicas to the primary region and @@ -441,6 +434,49 @@ func synthesizeVoterConstraints( } } +// synthesizeReplicaConstraints generates a ConstraintsConjunction clause +// representing the `constraints` field to be set for a multi-region database. +func synthesizeReplicaConstraints( + regions catpb.RegionNames, placement descpb.DataPlacement, +) ([]zonepb.ConstraintsConjunction, error) { + switch placement { + case descpb.DataPlacement_DEFAULT: + constraints := make([]zonepb.ConstraintsConjunction, len(regions)) + for i, region := range regions { + // Constrain at least 1 (voting or non-voting) replica per region. + constraints[i] = zonepb.ConstraintsConjunction{ + NumReplicas: 1, + Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(region)}, + } + } + return constraints, nil + case descpb.DataPlacement_RESTRICTED: + // In a RESTRICTED placement policy, the database zone config has no + // non-voters so that REGIONAL BY [TABLE | ROW] can inherit the RESTRICTED + // placement. Voter placement will be set at the table/partition level to + // the table/partition region. + + // NB: When setting empty constraints, use nil as opposed to []. When + // constraints are deserialized from the database, empty constraints are + // always deserialized as nil. Therefore, if constraints are set as [] here, + // the database will have a difference in its expected constraints vs the + // actual constraints when comparing using the multi-region validation + // builtins. + return nil, nil + default: + return nil, errors.AssertionFailedf("unknown data placement: %v", placement) + } +} + +// synthesizeLeasePreferences generates a LeasePreferences clause representing +// the `lease_preferences` field to be set for the primary region of a +// multi-region database or the home region of a table in such a database. +func synthesizeLeasePreferences(region catpb.RegionName) []zonepb.LeasePreference { + return []zonepb.LeasePreference{ + {Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(region)}}, + } +} + // zoneConfigForMultiRegionTable generates a ZoneConfig stub for a // regional-by-table or global table in a multi-region database. // @@ -457,89 +493,102 @@ func synthesizeVoterConstraints( // into an existing ZoneConfig. func zoneConfigForMultiRegionTable( localityConfig catpb.LocalityConfig, regionConfig multiregion.RegionConfig, -) (*zonepb.ZoneConfig, error) { - ret := zonepb.NewZoneConfig() +) (zonepb.ZoneConfig, error) { + zc := *zonepb.NewZoneConfig() switch l := localityConfig.Locality.(type) { case *catpb.LocalityConfig_Global_: // Enable non-blocking transactions. - ret.GlobalReads = proto.Bool(true) - - // For GLOBAL tables, we want non-voters in all regions - // for fast reads, so we have to manually build a zone config with the - // nonvoters as opposed to REGIONAL BY [TABLE | ROW] which can inherit the - // RESTRICTED placement from the database. - if regionConfig.IsPlacementRestricted() { - // We only care about NumVoters here at the table level. NumReplicas is set at - // the database level, not at the table/partition level. - numVoters, _ := getNumVotersAndNumReplicasForDefaultDatabaseRegions(regionConfig) - - ret.NumVoters = &numVoters - vc, err := synthesizeVoterConstraints(regionConfig.PrimaryRegion(), regionConfig) + zc.GlobalReads = proto.Bool(true) + + if !regionConfig.GlobalTablesInheritDatabaseConstraints() { + // For GLOBAL tables, we want non-voters in all regions for fast reads, so + // we always use a DEFAULT placement config, even if the database is using + // RESTRICTED placement. + regionConfig = regionConfig.WithPlacementDefault() + + numVoters, numReplicas := getNumVotersAndNumReplicas(regionConfig) + zc.NumVoters = &numVoters + zc.NumReplicas = &numReplicas + + constraints, err := synthesizeReplicaConstraints(regionConfig.Regions(), regionConfig.Placement()) if err != nil { - return nil, err + return zonepb.ZoneConfig{}, err } - ret.VoterConstraints = vc - - ret.InheritedConstraints = false - ret.NullVoterConstraintsIsEmpty = true - - numNonPrimaryRegions := len(regionConfig.Regions()) - 1 - // Placement only applies in zone survivability in which case voters are - // only in the primary region. This means the total number of replicas is - // numVotersForZoneSurvival voting replicas + 1 for each non-primary - // region. - ret.NumReplicas = proto.Int32(numVoters + int32(numNonPrimaryRegions)) - ret.Constraints = make([]zonepb.ConstraintsConjunction, len(regionConfig.Regions())) - for i, region := range regionConfig.Regions() { - ret.Constraints[i] = zonepb.ConstraintsConjunction{ - NumReplicas: 1, - Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(region)}, - } + zc.Constraints = constraints + zc.InheritedConstraints = false + + voterConstraints, err := synthesizeVoterConstraints(regionConfig.PrimaryRegion(), regionConfig) + if err != nil { + return zonepb.ZoneConfig{}, err } + zc.VoterConstraints = voterConstraints + zc.NullVoterConstraintsIsEmpty = true + + leasePreferences := synthesizeLeasePreferences(regionConfig.PrimaryRegion()) + zc.LeasePreferences = leasePreferences + zc.InheritedLeasePreferences = false + + zc = regionConfig.ApplyZoneConfigExtensionForGlobal(zc) } // Inherit lease preference from the database. We do // nothing here because `NewZoneConfig()` already marks the field as // 'inherited'. + return zc, nil case *catpb.LocalityConfig_RegionalByTable_: - primaryRegion := regionConfig.PrimaryRegion() + affinityRegion := regionConfig.PrimaryRegion() if l.RegionalByTable.Region != nil { - primaryRegion = *l.RegionalByTable.Region + affinityRegion = *l.RegionalByTable.Region } - regions := regionConfig.GetSuperRegionRegionsForRegion(primaryRegion) - if l.RegionalByTable.Region == nil && !regionConfig.IsMemberOfExplicitSuperRegion(primaryRegion) { - // If we don't have an explicit primary - // region, use the same configuration as the database and return a blank - // zcfg here. - return ret, nil + if l.RegionalByTable.Region == nil && !regionConfig.IsMemberOfExplicitSuperRegion(affinityRegion) { + // If we don't have an explicit affinity region, use the same + // configuration as the database and return a blank zcfg here. + return zc, nil } - numVoters, numReplicas := getNumVotersAndNumReplicas( - len(regions), regionConfig.SurvivalGoal(), regionConfig.IsPlacementRestricted(), - ) - ret.NumVoters = &numVoters + numVoters, numReplicas := getNumVotersAndNumReplicas(regionConfig) + zc.NumVoters = &numVoters - maybeAddConstraintsForSuperRegion(primaryRegion, regions, ret, numReplicas, regionConfig) + if regionConfig.IsMemberOfExplicitSuperRegion(affinityRegion) { + err := addConstraintsForSuperRegion(&zc, regionConfig, affinityRegion) + if err != nil { + return zonepb.ZoneConfig{}, err + } + } else if !regionConfig.RegionalInTablesInheritDatabaseConstraints(affinityRegion) { + // If the database constraints can't be inherited to serve as the + // constraints for this table, define the constraints ourselves. + zc.NumReplicas = &numReplicas - // If the table has a user-specified primary region, use it. - voterConstraints, err := synthesizeVoterConstraints(primaryRegion, regionConfig) + constraints, err := synthesizeReplicaConstraints(regionConfig.Regions(), regionConfig.Placement()) + if err != nil { + return zonepb.ZoneConfig{}, err + } + zc.Constraints = constraints + zc.InheritedConstraints = false + } + + // If the table has a user-specified affinity region, use it. + voterConstraints, err := synthesizeVoterConstraints(affinityRegion, regionConfig) if err != nil { - return nil, err + return zonepb.ZoneConfig{}, err } + zc.VoterConstraints = voterConstraints + zc.NullVoterConstraintsIsEmpty = true - ret.NullVoterConstraintsIsEmpty = true - ret.VoterConstraints = voterConstraints + leasePreferences := synthesizeLeasePreferences(affinityRegion) + zc.LeasePreferences = leasePreferences + zc.InheritedLeasePreferences = false - ret.InheritedLeasePreferences = false - ret.LeasePreferences = []zonepb.LeasePreference{ - {Constraints: []zonepb.Constraint{makeRequiredConstraintForRegion(primaryRegion)}}, - } + zc = regionConfig.ApplyZoneConfigExtensionForRegionalIn(zc, affinityRegion) + return zc, nil case *catpb.LocalityConfig_RegionalByRow_: // We purposely do not set anything here at table level - this should be done at // partition level instead. - return ret, nil + return zc, nil + default: + return zonepb.ZoneConfig{}, errors.AssertionFailedf( + "unexpected unknown locality type %T", localityConfig.Locality) } - return ret, nil } // applyZoneConfigForMultiRegionTableOption is an option that can be passed into @@ -633,7 +682,7 @@ func applyZoneConfigForMultiRegionTableOptionTableNewConfig( if err != nil { return false, zonepb.ZoneConfig{}, err } - zc.CopyFromZone(*localityZoneConfig, zonepb.MultiRegionZoneConfigFields) + zc.CopyFromZone(localityZoneConfig, zonepb.MultiRegionZoneConfigFields) return false, zc, nil } } @@ -665,7 +714,7 @@ var ApplyZoneConfigForMultiRegionTableOptionTableAndIndexes = func( // the want to ALTER the table locality. zc.ClearFieldsOfAllSubzones(zonepb.MultiRegionZoneConfigFields) - zc.CopyFromZone(*localityZoneConfig, zonepb.MultiRegionZoneConfigFields) + zc.CopyFromZone(localityZoneConfig, zonepb.MultiRegionZoneConfigFields) hasNewSubzones := table.IsLocalityRegionalByRow() if hasNewSubzones { @@ -1262,6 +1311,11 @@ func SynthesizeRegionConfig( return regionConfig, err } + zoneCfgExtensions, err := regionEnum.ZoneConfigExtensions() + if err != nil { + return regionConfig, err + } + transitioningRegionNames, err := regionEnum.TransitioningRegionNames() if err != nil { return regionConfig, err @@ -1279,6 +1333,7 @@ func SynthesizeRegionConfig( regionEnumID, dbDesc.GetRegionConfig().Placement, superRegions, + zoneCfgExtensions, multiregion.WithTransitioningRegions(transitioningRegionNames), ) diff --git a/pkg/sql/region_util_test.go b/pkg/sql/region_util_test.go index 609c079298e1..ab8e8993cf9f 100644 --- a/pkg/sql/region_util_test.go +++ b/pkg/sql/region_util_test.go @@ -17,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/multiregion" + "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/require" @@ -32,9 +33,17 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }{ { desc: "one region, zone survival", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + }, "region_a", - }, "region_a", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(3), NumVoters: proto.Int32(3), @@ -65,10 +74,18 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }, { desc: "two regions, zone survival", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_a", + }, "region_a", - }, "region_a", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(4), NumVoters: proto.Int32(3), @@ -105,11 +122,19 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }, { desc: "three regions, zone survival", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + }, "region_b", - "region_c", - "region_a", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(5), NumVoters: proto.Int32(3), @@ -152,11 +177,19 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }, { desc: "three regions, region survival", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + }, "region_b", - "region_c", - "region_a", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(5), NumVoters: proto.Int32(5), @@ -199,12 +232,20 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }, { desc: "four regions, zone survival", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(6), NumVoters: proto.Int32(3), @@ -253,12 +294,20 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }, { desc: "four regions, region survival", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(5), NumVoters: proto.Int32(5), @@ -308,9 +357,17 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }, { desc: "one region, restricted placement", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + }, "region_a", - }, "region_a", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_RESTRICTED, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_RESTRICTED, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(3), NumVoters: proto.Int32(3), @@ -334,12 +391,80 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }, { desc: "four regions, restricted placement", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + "region_d", + }, "region_a", - "region_b", - "region_c", - "region_d", - }, "region_a", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_RESTRICTED, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_RESTRICTED, + nil, + descpb.ZoneConfigExtensions{}, + ), + expected: zonepb.ZoneConfig{ + NumReplicas: proto.Int32(3), + NumVoters: proto.Int32(3), + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + }, + Constraints: nil, + NullVoterConstraintsIsEmpty: true, + VoterConstraints: []zonepb.ConstraintsConjunction{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + }, + }, + }, + { + // NOTE: this test case uses zone config extensions to mimic placement + // restricted. + desc: "four regions, zone survival, zone config extensions", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_a", + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + Regional: &zonepb.ZoneConfig{ + NumReplicas: proto.Int32(3), + Constraints: nil, + InheritedLeasePreferences: true, + }, + // Unused. Testing that this doesn't cause issues. + Global: &zonepb.ZoneConfig{ + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, + }, + }, + }, + }, + }, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(3), NumVoters: proto.Int32(3), @@ -361,6 +486,146 @@ func TestZoneConfigForMultiRegionDatabase(t *testing.T) { }, }, }, + { + // NOTE: this test case uses zone config extensions to mimic a + // database-level secondary region. + desc: "four regions, region survival, zone config extensions", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_b": { + NumReplicas: proto.Int32(6), + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + InheritedConstraints: true, + NullVoterConstraintsIsEmpty: true, + VoterConstraints: []zonepb.ConstraintsConjunction{ + { + NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + }, + // Unused. Testing that this doesn't cause issues. + "region_c": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + }, + }, + }, + // Unused. Testing that this doesn't cause issues. + Global: &zonepb.ZoneConfig{ + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, + }, + }, + }, + }, + }, + ), + expected: zonepb.ZoneConfig{ + NumReplicas: proto.Int32(6), + NumVoters: proto.Int32(5), + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + Constraints: []zonepb.ConstraintsConjunction{ + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, + }, + }, + }, + NullVoterConstraintsIsEmpty: true, + VoterConstraints: []zonepb.ConstraintsConjunction{ + { + NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + }, + }, } for _, tc := range testCases { @@ -392,12 +657,20 @@ func TestZoneConfigForMultiRegionTable(t *testing.T) { Global: &catpb.LocalityConfig_Global{}, }, }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ GlobalReads: proto.Bool(true), InheritedConstraints: true, @@ -411,12 +684,20 @@ func TestZoneConfigForMultiRegionTable(t *testing.T) { Global: &catpb.LocalityConfig_Global{}, }, }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ GlobalReads: proto.Bool(true), InheritedConstraints: true, @@ -424,166 +705,155 @@ func TestZoneConfigForMultiRegionTable(t *testing.T) { }, }, { - desc: "4-region regional by row table with zone survival", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByRow_{ - RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, - }, - }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), - expected: *(zonepb.NewZoneConfig()), - }, - { - desc: "4-region regional by row table with region survival", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByRow_{ - RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, - }, - }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), - expected: *(zonepb.NewZoneConfig()), - }, - { - desc: "4-region regional by table with zone survival on primary region", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByTable_{ - RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ - Region: nil, - }, - }, - }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), - expected: *(zonepb.NewZoneConfig()), - }, - { - desc: "4-region regional by table with regional survival on primary region", + desc: "4-region global table with restricted placement", localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByTable_{ - RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ - Region: nil, - }, - }, + Locality: &catpb.LocalityConfig_Global_{}, }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), - expected: *(zonepb.NewZoneConfig()), - }, - { - desc: "4-region regional by table with zone survival on non primary region", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByTable_{ - RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ - Region: protoRegionName("region_c"), - }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", }, - }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_RESTRICTED, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ - NumReplicas: nil, // Set at the database level. - NumVoters: proto.Int32(3), + NumReplicas: proto.Int32(6), + NumVoters: proto.Int32(3), + GlobalReads: proto.Bool(true), + NullVoterConstraintsIsEmpty: true, LeasePreferences: []zonepb.LeasePreference{ { Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, }, }, - InheritedConstraints: true, - NullVoterConstraintsIsEmpty: true, VoterConstraints: []zonepb.ConstraintsConjunction{ { Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, }, }, - }, - }, - { - desc: "4-region regional by table with regional survival on non primary region", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByTable_{ - RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ - Region: protoRegionName("region_c"), + Constraints: []zonepb.ConstraintsConjunction{ + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, }, - }, - }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), - expected: zonepb.ZoneConfig{ - NumReplicas: nil, // Set at the database level. - NumVoters: proto.Int32(5), - LeasePreferences: []zonepb.LeasePreference{ { + NumReplicas: 1, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, - }, - InheritedConstraints: true, - NullVoterConstraintsIsEmpty: true, - VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 2, + NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, }, }, }, }, }, { - desc: "4-region global table with restricted placement", + // NOTE: this test case uses zone config extensions to mimic a + // database-level secondary region. + desc: "4-region global table with zone config extensions (for global)", localityConfig: catpb.LocalityConfig{ Locality: &catpb.LocalityConfig_Global_{}, }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_RESTRICTED, nil), + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + Global: &zonepb.ZoneConfig{ + NumReplicas: proto.Int32(6), + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + InheritedConstraints: true, + NullVoterConstraintsIsEmpty: true, + VoterConstraints: []zonepb.ConstraintsConjunction{ + { + NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + }, + }, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(6), - NumVoters: proto.Int32(3), + NumVoters: proto.Int32(5), GlobalReads: proto.Bool(true), InheritedConstraints: false, NullVoterConstraintsIsEmpty: true, - InheritedLeasePreferences: true, + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, VoterConstraints: []zonepb.ConstraintsConjunction{ { + NumReplicas: 2, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, }, + { + NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, }, Constraints: []zonepb.ConstraintsConjunction{ { @@ -613,194 +883,495 @@ func TestZoneConfigForMultiRegionTable(t *testing.T) { }, }, }, - } - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - zc, err := zoneConfigForMultiRegionTable(tc.localityConfig, tc.regionConfig) - require.NoError(t, err) - require.Equal(t, tc.expected, *zc) - }) - } -} - -func TestZoneConfigForMultiRegionPartition(t *testing.T) { - defer leaktest.AfterTest(t)() - - testCases := []struct { - desc string - region catpb.RegionName - regionConfig multiregion.RegionConfig - expected zonepb.ZoneConfig - }{ { - desc: "4-region table with zone survivability", - region: "region_a", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + desc: "4-region global table with zone config extensions (for regional)", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_Global_{}, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + Regional: &zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + InheritedConstraints: true, + InheritedLeasePreferences: true, + }, + }, + ), expected: zonepb.ZoneConfig{ - NumReplicas: nil, // Set at the database level. - NumVoters: proto.Int32(3), - InheritedConstraints: true, + NumReplicas: proto.Int32(5), + NumVoters: proto.Int32(5), + GlobalReads: proto.Bool(true), NullVoterConstraintsIsEmpty: true, + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + }, VoterConstraints: []zonepb.ConstraintsConjunction{ { + NumReplicas: 2, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, }, }, - LeasePreferences: []zonepb.LeasePreference{ + Constraints: []zonepb.ConstraintsConjunction{ { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + { + NumReplicas: 1, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, + }, + }, }, }, }, { - desc: "4-region table with region survivability", - region: "region_a", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + desc: "4-region global table with zone config extensions (for regional in primary region)", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_Global_{}, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, descpb.InvalidID, descpb.DataPlacement_DEFAULT, nil), + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_b": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + InheritedConstraints: true, + }, + }, + }, + ), expected: zonepb.ZoneConfig{ - NumReplicas: nil, // Set at the database level. + NumReplicas: proto.Int32(5), NumVoters: proto.Int32(5), - InheritedConstraints: true, + GlobalReads: proto.Bool(true), NullVoterConstraintsIsEmpty: true, + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + }, VoterConstraints: []zonepb.ConstraintsConjunction{ { NumReplicas: 2, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, }, }, - LeasePreferences: []zonepb.LeasePreference{ + Constraints: []zonepb.ConstraintsConjunction{ + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + { + NumReplicas: 1, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, + }, + }, }, }, }, - } - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - zc, err := zoneConfigForMultiRegionPartition(tc.region, tc.regionConfig) - require.NoError(t, err) - require.Equal(t, tc.expected, zc) - }) - } -} - -func TestZoneConfigForRegionalByTableWithSuperRegions(t *testing.T) { - defer leaktest.AfterTest(t)() - - const validMultiRegionEnumID = 100 - - testCases := []struct { - desc string - localityConfig catpb.LocalityConfig - regionConfig multiregion.RegionConfig - expected zonepb.ZoneConfig - }{ { - desc: "super region with regional table, zone failure", + desc: "4-region global table with zone config extensions (for regional in non primary region)", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_Global_{}, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_c": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + }, + InheritedConstraints: true, + }, + }, + }, + ), + expected: zonepb.ZoneConfig{ + GlobalReads: proto.Bool(true), + InheritedConstraints: true, + InheritedLeasePreferences: true, + }, + }, + { + desc: "4-region regional by row table with zone survival", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_RegionalByRow_{ + RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, + }, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), + expected: *(zonepb.NewZoneConfig()), + }, + { + desc: "4-region regional by row table with region survival", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_RegionalByRow_{ + RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, + }, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), + expected: *(zonepb.NewZoneConfig()), + }, + { + desc: "4-region regional by table with zone survival on primary region", localityConfig: catpb.LocalityConfig{ Locality: &catpb.LocalityConfig_RegionalByTable_{ RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ - Region: protoRegionName("region_b"), + Region: nil, }, }, }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validMultiRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "super_region_ab", - Regions: catpb.RegionNames{"region_a", "region_b"}, + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), + expected: *(zonepb.NewZoneConfig()), + }, + { + desc: "4-region regional by table with regional survival on primary region", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_RegionalByTable_{ + RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ + Region: nil, + }, + }, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", }, - }), + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), + expected: *(zonepb.NewZoneConfig()), + }, + { + desc: "4-region regional by table with zone survival on non primary region", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_RegionalByTable_{ + RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ + Region: protoRegionName("region_c"), + }, + }, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ - NumReplicas: proto.Int32(4), - NumVoters: proto.Int32(3), - InheritedConstraints: false, + NumReplicas: nil, // Set at the database level. + NumVoters: proto.Int32(3), + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + InheritedConstraints: true, NullVoterConstraintsIsEmpty: true, - Constraints: []zonepb.ConstraintsConjunction{ + VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, + }, + }, + }, + { + desc: "4-region regional by table with regional survival on non primary region", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_RegionalByTable_{ + RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ + Region: protoRegionName("region_c"), + }, + }, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), + expected: zonepb.ZoneConfig{ + NumReplicas: nil, // Set at the database level. + NumVoters: proto.Int32(5), + LeasePreferences: []zonepb.LeasePreference{ { - NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, }, + InheritedConstraints: true, + NullVoterConstraintsIsEmpty: true, VoterConstraints: []zonepb.ConstraintsConjunction{ { + NumReplicas: 2, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, }, + }, + }, + { + desc: "4-region regional by table on non primary region with zone config extensions (for regional)", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_RegionalByTable_{ + RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ + Region: protoRegionName("region_c"), + }, + }, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + Regional: &zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + InheritedConstraints: true, + InheritedLeasePreferences: true, + }, + }, + ), + expected: zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + NumVoters: proto.Int32(5), LeasePreferences: []zonepb.LeasePreference{ { Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + InheritedConstraints: true, + NullVoterConstraintsIsEmpty: true, + VoterConstraints: []zonepb.ConstraintsConjunction{ + { + NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, }, }, }, { - desc: "super region with regional table region failure super region with 3 regions", + desc: "4-region regional by table on non primary region with zone config extensions (for regional in primary region)", localityConfig: catpb.LocalityConfig{ Locality: &catpb.LocalityConfig_RegionalByTable_{ RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ - Region: protoRegionName("region_b"), + Region: protoRegionName("region_c"), }, }, }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validMultiRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "super_region_abc", - Regions: catpb.RegionNames{"region_a", "region_b", "region_c"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", }, - }), + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_b": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + }, + InheritedConstraints: true, + }, + }, + }, + ), expected: zonepb.ZoneConfig{ - NumReplicas: proto.Int32(5), - NumVoters: proto.Int32(5), - InheritedConstraints: false, - NullVoterConstraintsIsEmpty: true, - Constraints: []zonepb.ConstraintsConjunction{ + NumReplicas: proto.Int32(5), + NumVoters: proto.Int32(5), + LeasePreferences: []zonepb.LeasePreference{ { - NumReplicas: 2, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, + }, + Constraints: []zonepb.ConstraintsConjunction{ { NumReplicas: 1, Constraints: []zonepb.Constraint{ @@ -813,87 +1384,166 @@ func TestZoneConfigForRegionalByTableWithSuperRegions(t *testing.T) { {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, - }, - VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 2, + NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + { + NumReplicas: 1, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, }, }, }, - LeasePreferences: []zonepb.LeasePreference{ + NullVoterConstraintsIsEmpty: true, + VoterConstraints: []zonepb.ConstraintsConjunction{ { + NumReplicas: 2, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, }, }, }, { - desc: "super region with regional table region failure super region with 4 regions", + desc: "4-region regional by table on non primary region with zone config extensions (for regional in non primary region)", localityConfig: catpb.LocalityConfig{ Locality: &catpb.LocalityConfig_RegionalByTable_{ RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ - Region: protoRegionName("region_b"), + Region: protoRegionName("region_c"), }, }, }, - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validMultiRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "super_region_abcd", - Regions: catpb.RegionNames{"region_a", "region_b", "region_c", "region_d"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", }, - }), + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_c": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + }, + InheritedConstraints: true, + }, + }, + }, + ), expected: zonepb.ZoneConfig{ - NumReplicas: proto.Int32(5), - NumVoters: proto.Int32(5), - InheritedConstraints: false, - NullVoterConstraintsIsEmpty: true, - Constraints: []zonepb.ConstraintsConjunction{ + NumReplicas: nil, // Set at the database level. + NumVoters: proto.Int32(5), + LeasePreferences: []zonepb.LeasePreference{ { - NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, { - NumReplicas: 1, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, }, + }, + InheritedConstraints: true, + NullVoterConstraintsIsEmpty: true, + VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 1, + NumReplicas: 2, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, + }, + }, + }, + { + desc: "4-region regional by table on non primary region with zone config extensions (for regional and for regional in non primary region)", + localityConfig: catpb.LocalityConfig{ + Locality: &catpb.LocalityConfig_RegionalByTable_{ + RegionalByTable: &catpb.LocalityConfig_RegionalByTable{ + Region: protoRegionName("region_c"), + }, + }, + }, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + Regional: &zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + InheritedConstraints: true, + InheritedLeasePreferences: true, + }, + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_c": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + }, + InheritedConstraints: true, + }, + }, + }, + ), + expected: zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + NumVoters: proto.Int32(5), + LeasePreferences: []zonepb.LeasePreference{ { - NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, - }, - VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 2, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, }, }, - LeasePreferences: []zonepb.LeasePreference{ + InheritedConstraints: true, + NullVoterConstraintsIsEmpty: true, + VoterConstraints: []zonepb.ConstraintsConjunction{ { + NumReplicas: 2, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, }, @@ -902,68 +1552,46 @@ func TestZoneConfigForRegionalByTableWithSuperRegions(t *testing.T) { } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - err := multiregion.ValidateRegionConfig(tc.regionConfig) - require.NoError(t, err) zc, err := zoneConfigForMultiRegionTable(tc.localityConfig, tc.regionConfig) require.NoError(t, err) - require.Equal(t, tc.expected, *zc) + require.Equal(t, tc.expected, zc) }) } } -func TestZoneConfigForRegionalByRowPartitionsWithSuperRegions(t *testing.T) { +func TestZoneConfigForMultiRegionPartition(t *testing.T) { defer leaktest.AfterTest(t)() - const validMultiRegionEnumID = 100 - testCases := []struct { - desc string - region catpb.RegionName - localityConfig catpb.LocalityConfig - regionConfig multiregion.RegionConfig - expected zonepb.ZoneConfig + desc string + region catpb.RegionName + regionConfig multiregion.RegionConfig + expected zonepb.ZoneConfig }{ { - desc: "super region with regional by row, zone failure, partition region_a", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByRow_{ - RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, - }, - }, + desc: "4-region table with zone survivability", region: "region_a", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validMultiRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "super_region_ab", - Regions: catpb.RegionNames{"region_a", "region_b"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", }, - }), + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ - NumReplicas: proto.Int32(4), + NumReplicas: nil, // Set at the database level. NumVoters: proto.Int32(3), - InheritedConstraints: false, + InheritedConstraints: true, NullVoterConstraintsIsEmpty: true, - Constraints: []zonepb.ConstraintsConjunction{ - { - NumReplicas: 1, - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, - }, - }, - { - NumReplicas: 1, - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, - }, - }, - }, VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 0, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, @@ -979,93 +1607,146 @@ func TestZoneConfigForRegionalByRowPartitionsWithSuperRegions(t *testing.T) { }, }, { - desc: "super region with regional by row, zone failure, partition region_b", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByRow_{ - RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, + desc: "4-region table with region survivability", + region: "region_a", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", }, - }, - region: "region_b", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_ZONE_FAILURE, validMultiRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "super_region_ab", - Regions: catpb.RegionNames{"region_a", "region_b"}, - }, - }), + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{}, + ), expected: zonepb.ZoneConfig{ - NumReplicas: proto.Int32(4), - NumVoters: proto.Int32(3), - InheritedConstraints: false, + NumReplicas: nil, // Set at the database level. + NumVoters: proto.Int32(5), + InheritedConstraints: true, NullVoterConstraintsIsEmpty: true, - Constraints: []zonepb.ConstraintsConjunction{ + VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 1, + NumReplicas: 2, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, + }, + LeasePreferences: []zonepb.LeasePreference{ { - NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, }, + }, + }, + { + desc: "4-region table with zone config extensions (for regional)", + region: "region_a", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + Regional: &zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + InheritedConstraints: true, + InheritedLeasePreferences: true, + }, + }, + ), + expected: zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + NumVoters: proto.Int32(5), + InheritedConstraints: true, + NullVoterConstraintsIsEmpty: true, VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 0, + NumReplicas: 2, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, }, LeasePreferences: []zonepb.LeasePreference{ { Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, }, }, }, { - desc: "super region with regional by row, region failure, partition region_a", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByRow_{ - RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, - }, - }, + desc: "4-region table with zone config extensions (for regional in primary region)", region: "region_a", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ - "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validMultiRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "super_region_abc", - Regions: catpb.RegionNames{"region_a", "region_b", "region_c"}, + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", }, - }), + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_b": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + }, + InheritedConstraints: true, + }, + }, + }, + ), expected: zonepb.ZoneConfig{ NumReplicas: proto.Int32(5), NumVoters: proto.Int32(5), - InheritedConstraints: false, NullVoterConstraintsIsEmpty: true, - Constraints: []zonepb.ConstraintsConjunction{ + VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 1, + NumReplicas: 2, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, + }, + LeasePreferences: []zonepb.LeasePreference{ { - NumReplicas: 2, + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + }, + Constraints: []zonepb.ConstraintsConjunction{ + { + NumReplicas: 1, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, @@ -1076,77 +1757,142 @@ func TestZoneConfigForRegionalByRowPartitionsWithSuperRegions(t *testing.T) { {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, }, }, - }, - VoterConstraints: []zonepb.ConstraintsConjunction{ { - NumReplicas: 2, + NumReplicas: 1, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, - }, - LeasePreferences: []zonepb.LeasePreference{ { + NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, }, }, }, }, }, { - desc: "super region with regional by row, region failure, partition region_b", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByRow_{ - RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, + desc: "4-region table with zone config extensions (for regional in non primary region)", + region: "region_a", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", }, - }, - region: "region_b", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ "region_b", - "region_c", - "region_a", - "region_d", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validMultiRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "super_region_abc", - Regions: catpb.RegionNames{"region_a", "region_b", "region_c"}, - }, - }), + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_a": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + }, + InheritedConstraints: true, + }, + }, + }, + ), expected: zonepb.ZoneConfig{ - NumReplicas: proto.Int32(5), + NumReplicas: nil, // Set at the database level. NumVoters: proto.Int32(5), - InheritedConstraints: false, + InheritedConstraints: true, NullVoterConstraintsIsEmpty: true, - Constraints: []zonepb.ConstraintsConjunction{ + VoterConstraints: []zonepb.ConstraintsConjunction{ { NumReplicas: 2, Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, + }, + LeasePreferences: []zonepb.LeasePreference{ { - NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, { - NumReplicas: 1, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, }, }, }, + }, + }, + { + desc: "4-region table with zone config extensions (for regional and for regional in non primary region)", + region: "region_a", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_b", + "region_c", + "region_a", + "region_d", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, + descpb.InvalidID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + Regional: &zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + InheritedConstraints: true, + InheritedLeasePreferences: true, + }, + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_a": { + LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + }, + }, + }, + InheritedConstraints: true, + }, + }, + }, + ), + expected: zonepb.ZoneConfig{ + NumReplicas: proto.Int32(8), + NumVoters: proto.Int32(5), + InheritedConstraints: true, + NullVoterConstraintsIsEmpty: true, VoterConstraints: []zonepb.ConstraintsConjunction{ { NumReplicas: 2, Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, }, }, }, LeasePreferences: []zonepb.LeasePreference{ + { + Constraints: []zonepb.Constraint{ + {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, + }, + }, { Constraints: []zonepb.Constraint{ {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, @@ -1155,89 +1901,218 @@ func TestZoneConfigForRegionalByRowPartitionsWithSuperRegions(t *testing.T) { }, }, }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + zc, err := zoneConfigForMultiRegionPartition(tc.region, tc.regionConfig) + require.NoError(t, err) + require.Equal(t, tc.expected, zc) + }) + } +} + +func TestValidateSuperRegionConfig(t *testing.T) { + defer leaktest.AfterTest(t)() + + const validRegionEnumID = 100 + + testCases := []struct { + testName string + err string + regionConfig multiregion.RegionConfig + }{ { - desc: "super region with regional by row, region failure, 6 regions, super region with 5 regions", - localityConfig: catpb.LocalityConfig{ - Locality: &catpb.LocalityConfig_RegionalByRow_{ - RegionalByRow: &catpb.LocalityConfig_RegionalByRow{}, + testName: "region names within a super region should be sorted", + err: "the regions within super region sr1 were not in a sorted order", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", }, - }, - region: "region_b", - regionConfig: multiregion.MakeRegionConfig(catpb.RegionNames{ "region_b", - "region_c", - "region_a", - "region_d", - "region_e", - "region_f", - }, "region_b", descpb.SurvivalGoal_REGION_FAILURE, validMultiRegionEnumID, descpb.DataPlacement_DEFAULT, []descpb.SuperRegion{ - { - SuperRegionName: "super_region_abcde", - Regions: catpb.RegionNames{"region_a", "region_b", "region_c", "region_d", "region_e"}, - }, - }), - expected: zonepb.ZoneConfig{ - NumReplicas: proto.Int32(6), - NumVoters: proto.Int32(5), - InheritedConstraints: false, - NullVoterConstraintsIsEmpty: true, - Constraints: []zonepb.ConstraintsConjunction{ + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ { - NumReplicas: 1, - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_a"}, - }, + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_b", "region_a"}, }, + }, + descpb.ZoneConfigExtensions{}), + }, + { + testName: "regions should be unique within a super region", + err: "duplicate region region_b found in super region sr1", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ { - NumReplicas: 1, - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, - }, + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_b", "region_b"}, + }, + }, + descpb.ZoneConfigExtensions{}), + }, + { + testName: "regions within a super region should map to a valid region on the database", + err: "region region_d not part of database", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_d"}, }, + }, + descpb.ZoneConfigExtensions{}), + }, + { + testName: "super region names should be sorted", + err: "super regions are not in sorted order based on the super region name", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ { - NumReplicas: 1, - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_c"}, - }, + SuperRegionName: "sr2", + Regions: []catpb.RegionName{"region_a"}, }, { - NumReplicas: 1, - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_d"}, - }, + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_b"}, + }, + }, + descpb.ZoneConfigExtensions{}), + }, + { + testName: "a region can only appear in one super region", + err: "region region_a found in multiple super regions", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_a"}, }, { - NumReplicas: 1, - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_e"}, - }, + SuperRegionName: "sr2", + Regions: []catpb.RegionName{"region_a"}, }, }, - VoterConstraints: []zonepb.ConstraintsConjunction{ + descpb.ZoneConfigExtensions{}), + }, + { + testName: "super region names must be unique", + err: "duplicate super regions with name sr1 found", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ { - NumReplicas: 2, - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, - }, + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_a"}, + }, + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_a"}, }, }, - LeasePreferences: []zonepb.LeasePreference{ + descpb.ZoneConfigExtensions{}), + }, + { + testName: "a super region should have at least one region", + err: "no regions found within super region sr1", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ { - Constraints: []zonepb.Constraint{ - {Type: zonepb.Constraint_REQUIRED, Key: "region", Value: "region_b"}, - }, + SuperRegionName: "sr1", + Regions: []catpb.RegionName{}, }, }, - }, + descpb.ZoneConfigExtensions{}), + }, + { + testName: "a super region should have at least three regions if the survival mode is region failure", + err: "super region sr1 only has 2 regions: at least 3 regions are required for surviving a region failure", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{ + "region_a", + "region_b", + "region_c", + }, + "region_b", + descpb.SurvivalGoal_REGION_FAILURE, validRegionEnumID, + descpb.DataPlacement_DEFAULT, + []descpb.SuperRegion{ + { + SuperRegionName: "sr1", + Regions: []catpb.RegionName{"region_a", "region_b"}, + }, + }, + descpb.ZoneConfigExtensions{}), }, } + for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - err := multiregion.ValidateRegionConfig(tc.regionConfig) - require.NoError(t, err) - zc, err := zoneConfigForMultiRegionPartition(tc.region, tc.regionConfig) - require.NoError(t, err) - require.Equal(t, tc.expected, zc) - }) + err := multiregion.ValidateRegionConfig(tc.regionConfig) + + require.Error(t, err) + require.True( + t, + testutils.IsError(err, tc.err), + "test %s: expected err %v, got %v", + tc.testName, + tc.err, + err, + ) } } From 918b22bcfa678ce10aab55d8ef7ebec29010cbda Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Fri, 8 Apr 2022 02:16:31 -0400 Subject: [PATCH 2/2] sql: add basic validation for zone config extensions This commit adds basic validation for zone config extensions. Currently, we only check that all per-region extensions map to a region on the RegionConfig. In the future, we can add more zone config extension validation to ensure zone config extensions do not subvert other portions of the multi-region config (e.g. like breaking REGION survivability). --- pkg/sql/catalog/multiregion/BUILD.bazel | 2 + pkg/sql/catalog/multiregion/region_config.go | 85 +++++++++++++------ .../catalog/multiregion/region_config_test.go | 19 +++++ pkg/sql/catalog/typedesc/type_desc.go | 9 +- 4 files changed, 86 insertions(+), 29 deletions(-) diff --git a/pkg/sql/catalog/multiregion/BUILD.bazel b/pkg/sql/catalog/multiregion/BUILD.bazel index 08f5829ceb0b..70c4b417f640 100644 --- a/pkg/sql/catalog/multiregion/BUILD.bazel +++ b/pkg/sql/catalog/multiregion/BUILD.bazel @@ -25,10 +25,12 @@ go_test( srcs = ["region_config_test.go"], deps = [ ":multiregion", + "//pkg/config/zonepb", "//pkg/sql/catalog/catpb", "//pkg/sql/catalog/descpb", "//pkg/testutils", "//pkg/util/leaktest", "@com_github_stretchr_testify//require", + "@org_golang_google_protobuf//proto", ], ) diff --git a/pkg/sql/catalog/multiregion/region_config.go b/pkg/sql/catalog/multiregion/region_config.go index 6ffd2391e0bf..f4d0bc08c49f 100644 --- a/pkg/sql/catalog/multiregion/region_config.go +++ b/pkg/sql/catalog/multiregion/region_config.go @@ -146,6 +146,12 @@ func (r *RegionConfig) SuperRegions() []descpb.SuperRegion { return r.superRegions } +// ZoneConfigExtensions returns the zone configuration extensions applied to the +// database. +func (r *RegionConfig) ZoneConfigExtensions() descpb.ZoneConfigExtensions { + return r.zoneCfgExtensions +} + // ApplyZoneConfigExtensionForGlobal applies the global table zone configuration // extensions to the provided zone configuration, returning the updated config. func (r *RegionConfig) ApplyZoneConfigExtensionForGlobal(zc zonepb.ZoneConfig) zonepb.ZoneConfig { @@ -288,8 +294,20 @@ func ValidateRegionConfig(config RegionConfig) error { "cannot have a database with restricted placement that is also region survivable") } - err := ValidateSuperRegions(config.SuperRegions(), config.SurvivalGoal(), config.Regions(), func(err error) error { + var err error + ValidateSuperRegions(config.SuperRegions(), config.SurvivalGoal(), config.Regions(), func(validateErr error) { + if err == nil { + err = validateErr + } + }) + if err != nil { return err + } + + ValidateZoneConfigExtensions(config.Regions(), config.ZoneConfigExtensions(), func(validateErr error) { + if err == nil { + err = validateErr + } }) if err != nil { return err @@ -307,8 +325,8 @@ func ValidateSuperRegions( superRegions []descpb.SuperRegion, survivalGoal descpb.SurvivalGoal, regionNames catpb.RegionNames, - errorHandler func(error) error, -) error { + errorHandler func(error), +) { seenRegions := make(map[catpb.RegionName]struct{}) superRegionNames := make(map[string]struct{}) @@ -317,32 +335,24 @@ func ValidateSuperRegions( return superRegions[i].SuperRegionName < superRegions[j].SuperRegionName }) { err := errors.AssertionFailedf("super regions are not in sorted order based on the super region name %v", superRegions) - if err := errorHandler(err); err != nil { - return err - } + errorHandler(err) } for _, superRegion := range superRegions { if len(superRegion.Regions) == 0 { err := errors.AssertionFailedf("no regions found within super region %s", superRegion.SuperRegionName) - if err := errorHandler(err); err != nil { - return err - } + errorHandler(err) } if err := CanSatisfySurvivalGoal(survivalGoal, len(superRegion.Regions)); err != nil { err := errors.HandleAsAssertionFailure(errors.Wrapf(err, "super region %s only has %d regions", superRegion.SuperRegionName, len(superRegion.Regions))) - if err := errorHandler(err); err != nil { - return err - } + errorHandler(err) } _, found := superRegionNames[superRegion.SuperRegionName] if found { err := errors.AssertionFailedf("duplicate super regions with name %s found", superRegion.SuperRegionName) - if err := errorHandler(err); err != nil { - return err - } + errorHandler(err) } superRegionNames[superRegion.SuperRegionName] = struct{}{} @@ -351,9 +361,7 @@ func ValidateSuperRegions( return superRegion.Regions[i] < superRegion.Regions[j] }) { err := errors.AssertionFailedf("the regions within super region %s were not in a sorted order", superRegion.SuperRegionName) - if err := errorHandler(err); err != nil { - return err - } + errorHandler(err) } seenRegionsInCurrentSuperRegion := make(map[catpb.RegionName]struct{}) @@ -361,14 +369,15 @@ func ValidateSuperRegions( _, found := seenRegionsInCurrentSuperRegion[region] if found { err := errors.AssertionFailedf("duplicate region %s found in super region %s", region, superRegion.SuperRegionName) - if err := errorHandler(err); err != nil { - return err - } + errorHandler(err) + continue } seenRegionsInCurrentSuperRegion[region] = struct{}{} _, found = seenRegions[region] if found { - return errors.AssertionFailedf("region %s found in multiple super regions", region) + err := errors.AssertionFailedf("region %s found in multiple super regions", region) + errorHandler(err) + continue } seenRegions[region] = struct{}{} @@ -381,13 +390,37 @@ func ValidateSuperRegions( } if !found { err := errors.Newf("region %s not part of database", region) - if err := errorHandler(err); err != nil { - return err - } + errorHandler(err) } } } - return nil +} + +// ValidateZoneConfigExtensions validates that zone configuration extensions are +// coherent with the rest of the multi-region configuration. It validates that: +// 1. All per-region extensions map to a region on the RegionConfig. +// 2. TODO(nvanbenschoten): add more zone config extension validation in the +// future to ensure zone config extensions do not subvert other portions +// of the multi-region config (e.g. like breaking REGION survivability). +func ValidateZoneConfigExtensions( + regionNames catpb.RegionNames, + zoneCfgExtensions descpb.ZoneConfigExtensions, + errorHandler func(error), +) { + // Ensure that all per-region extensions map to a region on the RegionConfig. + for regionExt := range zoneCfgExtensions.RegionalIn { + found := false + for _, regionInDB := range regionNames { + if regionExt == regionInDB { + found = true + break + } + } + if !found { + errorHandler(errors.AssertionFailedf("region %s has REGIONAL IN "+ + "zone config extension, but is not a region in the database", regionExt)) + } + } } // IsMemberOfSuperRegion returns a boolean representing if the region is part diff --git a/pkg/sql/catalog/multiregion/region_config_test.go b/pkg/sql/catalog/multiregion/region_config_test.go index 6987b193c0d9..5ca772af8fec 100644 --- a/pkg/sql/catalog/multiregion/region_config_test.go +++ b/pkg/sql/catalog/multiregion/region_config_test.go @@ -13,12 +13,14 @@ package multiregion_test import ( "testing" + "github.com/cockroachdb/cockroach/pkg/config/zonepb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/multiregion" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" ) func TestValidateRegionConfig(t *testing.T) { @@ -84,6 +86,23 @@ func TestValidateRegionConfig(t *testing.T) { descpb.ZoneConfigExtensions{}, ), }, + { + err: "region region_d has REGIONAL IN zone config extension, but is not a region in the database", + regionConfig: multiregion.MakeRegionConfig( + catpb.RegionNames{"region_a", "region_b", "region_c"}, + "region_b", + descpb.SurvivalGoal_ZONE_FAILURE, + validRegionEnumID, + descpb.DataPlacement_DEFAULT, + nil, + descpb.ZoneConfigExtensions{ + RegionalIn: map[catpb.RegionName]zonepb.ZoneConfig{ + "region_b": {NumReplicas: proto.Int32(7)}, + "region_d": {NumReplicas: proto.Int32(8)}, + }, + }, + ), + }, } for _, tc := range testCases { diff --git a/pkg/sql/catalog/typedesc/type_desc.go b/pkg/sql/catalog/typedesc/type_desc.go index 9387636c8514..7a0b04560a25 100644 --- a/pkg/sql/catalog/typedesc/type_desc.go +++ b/pkg/sql/catalog/typedesc/type_desc.go @@ -751,14 +751,17 @@ func (desc *immutable) validateMultiRegion( if err != nil { vea.Report(err) } - - err = multiregion.ValidateSuperRegions(superRegions, dbDesc.GetRegionConfig().SurvivalGoal, regionNames, func(err error) error { + multiregion.ValidateSuperRegions(superRegions, dbDesc.GetRegionConfig().SurvivalGoal, regionNames, func(err error) { vea.Report(err) - return nil }) + + zoneCfgExtensions, err := desc.ZoneConfigExtensions() if err != nil { vea.Report(err) } + multiregion.ValidateZoneConfigExtensions(regionNames, zoneCfgExtensions, func(err error) { + vea.Report(err) + }) } // ValidateTxnCommit implements the catalog.Descriptor interface.