Skip to content

Commit

Permalink
Merge #62008
Browse files Browse the repository at this point in the history
62008: sql: Change to using a session variable instead of FORCE syntax for overriding zone configuration settings for MR databases and tables r=otan a=ajstorm

sql: Block zone config updates to multi-region tables

Block users from updating the zone configurations of multi-region
tables, without first having them set the session variable
`override_multi_region_zone_config` to true. We block updates to
multi-region table/partition/index zone configurations because we don't
want users to accidentally override the prescribed settings, leaving
them open to sub-optimal performance. Note that only the multi-region
fields of the zone configuration are blocked behind this variable. All
other fields (gc.ttlseconds, range_min/max_bytes, etc) can be updated
without overriding.

Release note (sql change): Block users from updating the zone
configurations of multi-region tables.

sql: Use session variable instead of FORCE to override zone configs

With #61499 we introduced new syntax (FORCE) to override setting the
zone configurations on multi-region databases. Upon further reflection,
it was decided that a session variable would be better suited to the
task. This commit pulls out the FORCE syntax and replaces it with the
use of override_multi_region_zone_config;

Release note (sql change): Revert the release notes on #61499.  We now
use a session variable (override_multi_region_zone_config) to override
the zone configuration on multi-region databases (and tables).

Resolves: #57668.

Note to reviewers: Please ignore the first commit, as it's being separately reviewed as part of #61889.  That commit was pulle out into a separate PR as it's a release blocker, and there was a need to get it in urgently.

Co-authored-by: Adam Storm <storm@cockroachlabs.com>
  • Loading branch information
craig[bot] and ajstorm committed Mar 16, 2021
2 parents e0b59da + 3c2d556 commit f5ed0e2
Show file tree
Hide file tree
Showing 23 changed files with 657 additions and 186 deletions.
6 changes: 3 additions & 3 deletions docs/generated/sql/bnf/alter_zone_database_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
alter_zone_database_stmt ::=
'ALTER' 'DATABASE' database_name 'CONFIGURE' 'ZONE' 'USING' variable '=' 'COPY' 'FROM' 'PARENT' ( ( ',' variable '=' value | ',' variable '=' 'COPY' 'FROM' 'PARENT' ) )* opt_force
| 'ALTER' 'DATABASE' database_name 'CONFIGURE' 'ZONE' 'USING' variable '=' value ( ( ',' variable '=' value | ',' variable '=' 'COPY' 'FROM' 'PARENT' ) )* opt_force
| 'ALTER' 'DATABASE' database_name 'CONFIGURE' 'ZONE' 'DISCARD' opt_force
'ALTER' 'DATABASE' database_name 'CONFIGURE' 'ZONE' 'USING' variable '=' 'COPY' 'FROM' 'PARENT' ( ( ',' variable '=' value | ',' variable '=' 'COPY' 'FROM' 'PARENT' ) )*
| 'ALTER' 'DATABASE' database_name 'CONFIGURE' 'ZONE' 'USING' variable '=' value ( ( ',' variable '=' value | ',' variable '=' 'COPY' 'FROM' 'PARENT' ) )*
| 'ALTER' 'DATABASE' database_name 'CONFIGURE' 'ZONE' 'DISCARD'
14 changes: 5 additions & 9 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -1729,7 +1729,7 @@ alter_rename_database_stmt ::=
'ALTER' 'DATABASE' database_name 'RENAME' 'TO' database_name

alter_zone_database_stmt ::=
'ALTER' 'DATABASE' database_name set_zone_config opt_force
'ALTER' 'DATABASE' database_name set_zone_config

alter_database_owner ::=
'ALTER' 'DATABASE' database_name 'OWNER' 'TO' role_spec
Expand All @@ -1738,16 +1738,16 @@ alter_database_to_schema_stmt ::=
'ALTER' 'DATABASE' database_name 'CONVERT' 'TO' 'SCHEMA' 'WITH' 'PARENT' database_name

alter_database_add_region_stmt ::=
'ALTER' 'DATABASE' database_name 'ADD' 'REGION' region_name opt_force
'ALTER' 'DATABASE' database_name 'ADD' 'REGION' region_name

alter_database_drop_region_stmt ::=
'ALTER' 'DATABASE' database_name 'DROP' 'REGION' region_name opt_force
'ALTER' 'DATABASE' database_name 'DROP' 'REGION' region_name

alter_database_survival_goal_stmt ::=
'ALTER' 'DATABASE' database_name survival_goal_clause opt_force
'ALTER' 'DATABASE' database_name survival_goal_clause

alter_database_primary_region_stmt ::=
'ALTER' 'DATABASE' database_name primary_region_clause opt_force
'ALTER' 'DATABASE' database_name primary_region_clause

alter_zone_range_stmt ::=
'ALTER' 'RANGE' zone_name set_zone_config
Expand Down Expand Up @@ -2238,10 +2238,6 @@ alter_index_cmds ::=
sequence_option_list ::=
( sequence_option_elem ) ( ( sequence_option_elem ) )*

opt_force ::=
'FORCE'
|

region_name ::=
name

Expand Down
6 changes: 3 additions & 3 deletions pkg/bench/ddl_analysis/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ exp,benchmark
39,AlterTableAddForeignKey/alter_table_add_2_foreign_keys
49,AlterTableAddForeignKey/alter_table_add_3_foreign_keys
29,AlterTableAddForeignKey/alter_table_add_foreign_key_with_3_columns
28,AlterTableConfigureZone/alter_table_configure_zone_5_replicas
28,AlterTableConfigureZone/alter_table_configure_zone_7_replicas_
28,AlterTableConfigureZone/alter_table_configure_zone_ranges
30,AlterTableConfigureZone/alter_table_configure_zone_5_replicas
30,AlterTableConfigureZone/alter_table_configure_zone_7_replicas_
30,AlterTableConfigureZone/alter_table_configure_zone_ranges
23,AlterTableDropColumn/alter_table_drop_1_column
27,AlterTableDropColumn/alter_table_drop_2_columns
31,AlterTableDropColumn/alter_table_drop_3_columns
Expand Down
12 changes: 7 additions & 5 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region
Original file line number Diff line number Diff line change
Expand Up @@ -1084,14 +1084,16 @@ txn_database_drop_regions ca-central-1 true {ca-az1,ca-az2,ca-az3}
txn_database_drop_regions ap-southeast-2 false {ap-az1,ap-az2,ap-az3}
txn_database_drop_regions us-east-1 false {us-az1,us-az2,us-az3}

# Adding a FORCE to this second statement until we get a fix for #60620. When
# that fix is ready, we can construct the view of the zone config as it was at
# the beginning of the transaction, and the checks for FORCE should work again,
# and we won't require the explicit force here.
# Overriding this operation until we get a fix for #60620. When that fix is
# ready, we can construct the view of the zone config as it was at the
# beginning of the transaction, and the checks for override should work
# again, and we won't require an explicit override here.
statement ok
BEGIN;
SET override_multi_region_zone_config = true;
ALTER DATABASE txn_database_drop_regions DROP REGION "us-east-1";
ALTER DATABASE txn_database_drop_regions DROP REGION "ap-southeast-2" FORCE;
ALTER DATABASE txn_database_drop_regions DROP REGION "ap-southeast-2";
SET override_multi_region_zone_config = false;
COMMIT;

query TTBT colnames
Expand Down
Loading

0 comments on commit f5ed0e2

Please sign in to comment.