Skip to content

Commit

Permalink
sql: block initial multi-region if zone configs have multi-region fie…
Browse files Browse the repository at this point in the history 
…lds set

Release note (sql change): Block being able to set the initial PRIMARY
REGION of a database if any multi-region fields on any zone configs in
the database have been set.
  • Loading branch information
@otan
otan committed Apr 6, 2021
1 parent b0bdb08 commit 48287b6
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 15 deletions.
84 changes: 80 additions & 4 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region_zone_configs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,13 +496,13 @@ SET override_multi_region_zone_config = true;
ALTER index regional_by_row@primary CONFIGURE ZONE USING num_replicas = 10;
SET override_multi_region_zone_config = false

statement error attempting to update zone config which contains an extra zone configuration for index regional_by_row@"primary"
statement error attempting to update zone config which contains an extra zone configuration for index regional_by_row@"primary" with field num_replicas populated
ALTER TABLE regional_by_row SET LOCALITY GLOBAL

statement error extraneous zone configuration for index regional_by_row@"primary"
statement error extraneous zone configuration for index regional_by_row@"primary" with field num_replicas populated
SELECT crdb_internal.validate_multi_region_zone_configs()

statement error attempting to update zone config which contains an extra zone configuration for index regional_by_row@"primary"
statement error attempting to update zone config which contains an extra zone configuration for index regional_by_row@"primary" with field num_replicas populated
ALTER TABLE regional_by_row SET LOCALITY REGIONAL BY ROW

statement ok
Expand Down Expand Up @@ -537,7 +537,7 @@ INDEX regional_by_row_as@primary ALTER INDEX regional_by_row_as@primary CONFIGU
voter_constraints = '[+region=us-east-1]',
lease_preferences = '[[+region=us-east-1]]'

statement error attempting to update zone config which contains an extra zone configuration for index regional_by_row_as@"primary"
statement error attempting to update zone config which contains an extra zone configuration for index regional_by_row_as@"primary" with field num_replicas populated
ALTER TABLE regional_by_row_as SET LOCALITY REGIONAL BY ROW

statement ok
Expand Down Expand Up @@ -641,3 +641,79 @@ statement ok
SET override_multi_region_zone_config = true;
ALTER PARTITION "ca-central-1" OF INDEX regional_by_row@primary CONFIGURE ZONE DISCARD;
SET override_multi_region_zone_config = false

# Test validation for initial SET PRIMARY REGION
statement ok
CREATE DATABASE initial_multiregion_db;
USE initial_multiregion_db;
CREATE TABLE tbl (a INT PRIMARY KEY, INDEX a_idx (a));
ALTER DATABASE initial_multiregion_db CONFIGURE ZONE USING gc.ttlseconds = 5;
ALTER TABLE tbl CONFIGURE ZONE USING gc.ttlseconds = 5;
ALTER INDEX tbl@a_idx CONFIGURE ZONE USING gc.ttlseconds = 5

statement ok
ALTER DATABASE initial_multiregion_db SET PRIMARY REGION "us-east-1"

query TT
SHOW ZONE CONFIGURATION FOR DATABASE initial_multiregion_db
----
DATABASE initial_multiregion_db ALTER DATABASE initial_multiregion_db CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 5,
num_replicas = 3,
num_voters = 3,
constraints = '{+region=us-east-1: 1}',
voter_constraints = '[+region=us-east-1]',
lease_preferences = '[[+region=us-east-1]]'

query TT
SHOW ZONE CONFIGURATION FOR TABLE tbl
----
TABLE tbl ALTER TABLE tbl CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 5,
num_replicas = 3,
num_voters = 3,
constraints = '{+region=us-east-1: 1}',
voter_constraints = '[+region=us-east-1]',
lease_preferences = '[[+region=us-east-1]]'

query TT
SHOW ZONE CONFIGURATION FOR INDEX tbl@a_idx
----
INDEX tbl@a_idx ALTER INDEX tbl@a_idx CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 5,
num_replicas = 3,
num_voters = 3,
constraints = '{+region=us-east-1: 1}',
voter_constraints = '[+region=us-east-1]',
lease_preferences = '[[+region=us-east-1]]'

statement ok
ALTER DATABASE initial_multiregion_db DROP REGION "us-east-1";
ALTER INDEX tbl@a_idx CONFIGURE ZONE USING num_replicas = 10

statement error zone configuration for index tbl@a_idx has field "num_replicas" set which will be overwritten when setting the initial PRIMARY REGION\nHINT: discard the zone config using CONFIGURE ZONE DISCARD before continuing
ALTER DATABASE initial_multiregion_db SET PRIMARY REGION "us-east-1"

statement ok
ALTER INDEX tbl@a_idx CONFIGURE ZONE DISCARD;
ALTER TABLE tbl CONFIGURE ZONE USING num_replicas = 10

statement error zone configuration for table tbl has field "num_replicas" set which will be overwritten when setting the the initial PRIMARY REGION\nHINT: discard the zone config using CONFIGURE ZONE DISCARD before continuing
ALTER DATABASE initial_multiregion_db SET PRIMARY REGION "us-east-1"

statement ok
ALTER TABLE tbl CONFIGURE ZONE DISCARD;
ALTER DATABASE initial_multiregion_db CONFIGURE ZONE USING num_replicas = 10;

statement error zone configuration for database initial_multiregion_db has field "num_replicas" set which will be overwritten when setting the the initial PRIMARY REGION\nHINT: discard the zone config using CONFIGURE ZONE DISCARD before continuing
ALTER DATABASE initial_multiregion_db SET PRIMARY REGION "us-east-1"

statement ok
ALTER DATABASE initial_multiregion_db CONFIGURE ZONE DISCARD;
ALTER DATABASE initial_multiregion_db SET PRIMARY REGION "us-east-1"
4 changes: 2 additions & 2 deletions pkg/config/zonepb/zone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,8 +611,6 @@ type DiffWithZoneMismatch struct {
// PartitionName represents a subzone with a mismatching partitionName.
PartitionName string

// NOTE: only one of the below fields is set.

// IsMissingSubzone indicates a subzone is missing.
IsMissingSubzone bool
// IsExtraSubzone indicates we have an extraneous subzone.
Expand Down Expand Up @@ -782,6 +780,7 @@ func (z *ZoneConfig) DiffWithZone(
IndexID: s.IndexID,
PartitionName: s.PartitionName,
IsExtraSubzone: true,
Field: subzoneMismatch.Field,
}, nil
}
continue
Expand Down Expand Up @@ -822,6 +821,7 @@ func (z *ZoneConfig) DiffWithZone(
IndexID: o.IndexID,
PartitionName: o.PartitionName,
IsMissingSubzone: true,
Field: subzoneMismatch.Field,
}, nil
}
}
Expand Down
10 changes: 9 additions & 1 deletion pkg/sql/alter_database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,6 @@ func checkCanConvertTableToMultiRegion(
)
}
}
// TODO(#57668): check zone configurations are not set here
return nil
}

Expand All @@ -775,6 +774,15 @@ func (n *alterDatabasePrimaryRegionNode) setInitialPrimaryRegion(params runParam
return err
}

// Check we are writing valid zone configurations.
if err := params.p.validateAllMultiRegionZoneConfigsInDatabase(
params.ctx,
&n.desc.Immutable,
&zoneConfigForMultiRegionValidatorSetInitialRegion{},
); err != nil {
return err
}

// Set the region config on the database descriptor.
if err := n.desc.SetInitialMultiRegionConfig(regionConfig); err != nil {
return err
Expand Down
91 changes: 83 additions & 8 deletions pkg/sql/region_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1169,8 +1169,79 @@ type zoneConfigForMultiRegionValidator interface {
getExpectedTableZoneConfig(desc catalog.TableDescriptor) (zonepb.ZoneConfig, error)

newMismatchFieldError(descType string, descName string, field string) error
newMissingSubzoneError(descType string, descName string) error
newExtraSubzoneError(descType string, descName string) error
newMissingSubzoneError(descType string, descName string, field string) error
newExtraSubzoneError(descType string, descName string, field string) error
}

// zoneConfigForMultiRegionValidatorSetInitialRegion implements
// interface zoneConfigForMultiRegionValidator.
type zoneConfigForMultiRegionValidatorSetInitialRegion struct{}

var _ zoneConfigForMultiRegionValidator = (*zoneConfigForMultiRegionValidatorSetInitialRegion)(nil)

func (v *zoneConfigForMultiRegionValidatorSetInitialRegion) getExpectedDatabaseZoneConfig() (
zonepb.ZoneConfig,
error,
) {
return *zonepb.NewZoneConfig(), nil
}

func (v *zoneConfigForMultiRegionValidatorSetInitialRegion) getExpectedTableZoneConfig(
desc catalog.TableDescriptor,
) (zonepb.ZoneConfig, error) {
return *zonepb.NewZoneConfig(), nil
}

func (v *zoneConfigForMultiRegionValidatorSetInitialRegion) wrapErr(err error) error {
// We currently do not allow "inherit from parent" behavior, so one must
// discard the zone config before continuing.
// COPY FROM PARENT copies the value but does not inherit.
// This can be replaced with the override session variable hint when it is
// available.
return errors.WithHintf(
err,
"discard the zone config using CONFIGURE ZONE DISCARD before continuing",
)
}

func (v *zoneConfigForMultiRegionValidatorSetInitialRegion) newMismatchFieldError(
descType string, descName string, field string,
) error {
return v.wrapErr(
pgerror.Newf(
pgcode.InvalidObjectDefinition,
"zone configuration for %s %s has field %q set which will be overwritten when setting the the initial PRIMARY REGION",
descType,
descName,
field,
),
)
}

func (v *zoneConfigForMultiRegionValidatorSetInitialRegion) newMissingSubzoneError(
descType string, descName string, field string,
) error {
// There can never be a missing subzone as we only compare against
// blank zone configs.
return errors.AssertionFailedf(
"unexpected missing subzone for %s %s",
descType,
descName,
)
}

func (v *zoneConfigForMultiRegionValidatorSetInitialRegion) newExtraSubzoneError(
descType string, descName string, field string,
) error {
return v.wrapErr(
pgerror.Newf(
pgcode.InvalidObjectDefinition,
"zone configuration for %s %s has field %q set which will be overwritten when setting the initial PRIMARY REGION",
descType,
descName,
field,
),
)
}

// zoneConfigForMultiRegionValidatorExistingMultiRegionObject partially implements
Expand Down Expand Up @@ -1235,7 +1306,7 @@ func (v *zoneConfigForMultiRegionValidatorModifiedByUser) wrapErr(err error) err
}

func (v *zoneConfigForMultiRegionValidatorModifiedByUser) newMissingSubzoneError(
descType string, descName string,
descType string, descName string, field string,
) error {
return v.wrapErr(
pgerror.Newf(
Expand All @@ -1248,14 +1319,15 @@ func (v *zoneConfigForMultiRegionValidatorModifiedByUser) newMissingSubzoneError
}

func (v *zoneConfigForMultiRegionValidatorModifiedByUser) newExtraSubzoneError(
descType string, descName string,
descType string, descName string, field string,
) error {
return v.wrapErr(
pgerror.Newf(
pgcode.InvalidObjectDefinition,
"attempting to update zone config which contains an extra zone configuration for %s %s",
"attempting to update zone config which contains an extra zone configuration for %s %s with field %s populated",
descType,
descName,
field,
),
)
}
Expand All @@ -1281,7 +1353,7 @@ func (v *zoneConfigForMultiRegionValidatorValidation) newMismatchFieldError(
}

func (v *zoneConfigForMultiRegionValidatorValidation) newMissingSubzoneError(
descType string, descName string,
descType string, descName string, field string,
) error {
return pgerror.Newf(
pgcode.InvalidObjectDefinition,
Expand All @@ -1292,13 +1364,14 @@ func (v *zoneConfigForMultiRegionValidatorValidation) newMissingSubzoneError(
}

func (v *zoneConfigForMultiRegionValidatorValidation) newExtraSubzoneError(
descType string, descName string,
descType string, descName string, field string,
) error {
return pgerror.Newf(
pgcode.InvalidObjectDefinition,
"extraneous zone configuration for %s %s",
"extraneous zone configuration for %s %s with field %s populated",
descType,
descName,
field,
)
}

Expand Down Expand Up @@ -1505,12 +1578,14 @@ func (p *planner) validateZoneConfigForMultiRegionTable(
return zoneConfigForMultiRegionValidator.newMissingSubzoneError(
descType,
name,
mismatch.Field,
)
}
if mismatch.IsExtraSubzone {
return zoneConfigForMultiRegionValidator.newExtraSubzoneError(
descType,
name,
mismatch.Field,
)
}
return zoneConfigForMultiRegionValidator.newMismatchFieldError(
Expand Down

0 comments on commit 48287b6

Please sign in to comment.