Merge cockroachdb#87108 cockroachdb#87349 cockroachdb#87411

87108: sql: add secondary region implicitly r=e-mbrown a=e-mbrown fixes cockroachdb#86880 This commit adds the secondary region onto the database when it is declared. Release justification: Low risk changes to existing functionality Release note (sql change): The SECONDARY REGION is added onto the db, when it is declared. The region assigned as the SECONDARY REGION doesn't need to be explicitly listed in REGIONS 87349: cloud/amazon: add params in put-upload case r=dt a=dt Previously the setting enabled this alternative write path, which exactly matched the write behavior of 21.1. However, 21.1 did not have options for server-side encryption or storage class, while more recent versions do, and thus this alternative write path should respect them if set. Release note: none. Release justification: none (off by default). 87411: sql/schemachanger: add creation time to indexes r=ajwerner a=ajwerner This fixes a gap in the declarative schema changer whereby it did not previously populate the `CreatedAtNanos` field of new secondary indexes. The declarative schema changer is not used to create many secondary indexes, so it was not a huge deal, but, nevertheless, the oversight has been fixed. Fixes cockroachdb#86196 Release justification: Low risk change to new functionality Release note: None Co-authored-by: e-mbrown <[email protected]> Co-authored-by: David Taylor <[email protected]> Co-authored-by: Andrew Werner <[email protected]>
mgartner · Sep 6, 2022 · 77be6d8 · 77be6d8
4 parents af1a53a + 6011944 + 92eadd7 + f904303
commit 77be6d8
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 27 deletions.
diff --git a/pkg/ccl/logictestccl/testdata/logic_test/secondary_region b/pkg/ccl/logictestccl/testdata/logic_test/secondary_region
@@ -17,9 +17,23 @@ CREATE DATABASE non_mr;
 statement error database must be multi-region to support a secondary region
 ALTER DATABASE non_mr SET SECONDARY REGION "ap-southeast2"
 
-statement error pq: region .* has not been added to the database
+statement ok
 CREATE DATABASE no_list PRIMARY REGION "ap-southeast-2" SECONDARY REGION "ca-central-1"
 
+# Verify secondry region is added implicitly
+query TT
+SHOW ZONE CONFIGURATION FOR DATABASE no_list;
+----
+DATABASE no_list  ALTER DATABASE no_list CONFIGURE ZONE USING
+                  range_min_bytes = 134217728,
+                  range_max_bytes = 536870912,
+                  gc.ttlseconds = 90000,
+                  num_replicas = 4,
+                  num_voters = 3,
+                  constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1}',
+                  voter_constraints = '[+region=ap-southeast-2]',
+                  lease_preferences = '[[+region=ap-southeast-2, +region=ca-central-1]]'
+
 statement ok
 CREATE DATABASE db PRIMARY REGION "ap-southeast-2" REGIONS "ca-central-1" SECONDARY REGION "ca-central-1"
 
@@ -469,12 +483,12 @@ ALTER DATABASE mr1 ALTER SUPER REGION "test1" VALUES "us-east-1"
 query TTTTT colnames
 SHOW REGIONS
 ----
-region          zones                      database_names    primary_region_of  secondary_region_of
-ap-southeast-2  {ap-az1,ap-az2,ap-az3}     {db,mr1,mr2,mr3}  {db,mr2}           {}
-ca-central-1    {ca-az1,ca-az2,ca-az3}     {db,mr1,mr2,mr3}  {mr3}              {mr2}
-us-central-1    {usc-az1,usc-az2,usc-az3}  {mr1,mr2,mr3}     {}                 {mr3}
-us-east-1       {us-az1,us-az2,us-az3}     {db,mr1,mr2,mr3}  {mr1}              {}
-us-west-1       {usw-az1,usw-az2,usw-az3}  {mr1,mr2,mr3}     {}                 {mr1}
+region          zones                      database_names            primary_region_of  secondary_region_of
+ap-southeast-2  {ap-az1,ap-az2,ap-az3}     {db,mr1,mr2,mr3,no_list}  {db,mr2,no_list}   {}
+ca-central-1    {ca-az1,ca-az2,ca-az3}     {db,mr1,mr2,mr3,no_list}  {mr3}              {mr2,no_list}
+us-central-1    {usc-az1,usc-az2,usc-az3}  {mr1,mr2,mr3}             {}                 {mr3}
+us-east-1       {us-az1,us-az2,us-az3}     {db,mr1,mr2,mr3}          {mr1}              {}
+us-west-1       {usw-az1,usw-az2,usw-az3}  {mr1,mr2,mr3}             {}                 {mr1}
 
 query TTBBT colnames
 SHOW REGIONS FROM DATABASE mr1
@@ -515,6 +529,7 @@ defaultdb      {}                                                              N
 mr1            {ap-southeast-2,ca-central-1,us-central-1,us-east-1,us-west-1}  us-east-1       us-west-1
 mr2            {ap-southeast-2,ca-central-1,us-central-1,us-east-1,us-west-1}  ap-southeast-2  ca-central-1
 mr3            {ap-southeast-2,ca-central-1,us-central-1,us-east-1,us-west-1}  ca-central-1    us-central-1
+no_list        {ap-southeast-2,ca-central-1}                                   ap-southeast-2  ca-central-1
 non_mr         {}                                                              NULL            NULL
 postgres       {}                                                              NULL            NULL
 system         {}                                                              NULL            NULL

diff --git a/pkg/ccl/multiregionccl/multiregion.go b/pkg/ccl/multiregionccl/multiregion.go
@@ -95,6 +95,12 @@ func initializeMultiRegionMetadata(
 		regionNames = append(regionNames, primaryRegion)
 	}
 
+	if secondaryRegion != catpb.RegionName(tree.SecondaryRegionNotSpecifiedName) {
+		if _, ok := seenRegions[secondaryRegion]; !ok {
+			regionNames = append(regionNames, secondaryRegion)
+		}
+	}
+
 	sort.SliceStable(regionNames, func(i, j int) bool {
 		return regionNames[i] < regionNames[j]
 	})

diff --git a/pkg/ccl/schemachangerccl/testdata/end_to_end/create_index b/pkg/ccl/schemachangerccl/testdata/end_to_end/create_index
@@ -24,6 +24,7 @@ upsert descriptor #104
   +  mutations:
   +  - direction: ADD
   +    index:
+  +      createdAtNanos: "1640998800000000000"
   +      createdExplicitly: true
   +      foreignKey: {}
   +      geoConfig: {}
@@ -261,7 +262,8 @@ upsert descriptor #104
   -  modificationTime:
   -    wallTime: "1640995200000000008"
   +  indexes:
-  +  - createdExplicitly: true
+  +  - createdAtNanos: "1640998800000000000"
+  +    createdExplicitly: true
   +    foreignKey: {}
   +    geoConfig: {}
   +    id: 2
@@ -294,6 +296,7 @@ upsert descriptor #104
   -  - direction: ADD
   +  - direction: DROP
        index:
+  -      createdAtNanos: "1640998800000000000"
          createdExplicitly: true
          foreignKey: {}
          geoConfig: {}

diff --git a/pkg/cloud/amazon/s3_storage.go b/pkg/cloud/amazon/s3_storage.go
@@ -578,8 +578,11 @@ func (s *s3Storage) putUploader(ctx context.Context, basename string) (io.WriteC
 	return &putUploader{
 		b: buf,
 		input: &s3.PutObjectInput{
-			Bucket: s.bucket,
-			Key:    aws.String(path.Join(s.prefix, basename)),
+			Bucket:               s.bucket,
+			Key:                  aws.String(path.Join(s.prefix, basename)),
+			ServerSideEncryption: nilIfEmpty(s.conf.ServerEncMode),
+			SSEKMSKeyId:          nilIfEmpty(s.conf.ServerKMSID),
+			StorageClass:         nilIfEmpty(s.conf.StorageClass),
 		},
 		client: client,
 	}, nil

diff --git a/pkg/sql/create_database.go b/pkg/sql/create_database.go
@@ -149,21 +149,6 @@ func (p *planner) CreateDatabase(ctx context.Context, n *tree.CreateDatabase) (p
 		)
 	}
 
-	if n.SecondaryRegion != tree.SecondaryRegionNotSpecifiedName {
-		if !n.Regions.Contains(n.SecondaryRegion) {
-			return nil, errors.WithHintf(
-				pgerror.Newf(pgcode.InvalidName,
-					"region %s has not been added to the database",
-					n.SecondaryRegion.String(),
-				),
-				"you must add the region to the database before setting it as primary region, using "+
-					"ALTER DATABASE %s ADD REGION %s",
-				n.Name.String(),
-				n.SecondaryRegion.String(),
-			)
-		}
-	}
-
 	return &createDatabaseNode{n: n}, nil
 }
 

diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/index.go b/pkg/sql/schemachanger/scexec/scmutationexec/index.go
@@ -91,6 +91,9 @@ func addNewIndexMutation(
 		UseDeletePreservingEncoding: isDeletePreserving,
 		StoreColumnNames:            []string{},
 	}
+	if isSecondary && !isDeletePreserving {
+		idx.CreatedAtNanos = m.clock.ApproximateTime().UnixNano()
+	}
 	if opIndex.Sharding != nil {
 		idx.Sharded = *opIndex.Sharding
 	}

diff --git a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_alter_primary_key_vanilla b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_alter_primary_key_vanilla
@@ -78,6 +78,7 @@ upsert descriptor #104
   +  - direction: ADD
   +    index:
   +      constraintId: 4
+  +      createdAtNanos: "1640998800000000000"
   +      createdExplicitly: true
   +      foreignKey: {}
   +      geoConfig: {}
@@ -327,6 +328,7 @@ upsert descriptor #104
   -    wallTime: "1640995200000000008"
   +  indexes:
   +  - constraintId: 4
+  +    createdAtNanos: "1640998800000000000"
   +    createdExplicitly: true
   +    foreignKey: {}
   +    geoConfig: {}
@@ -388,6 +390,7 @@ upsert descriptor #104
   +  - direction: DROP
        index:
   -      constraintId: 4
+  -      createdAtNanos: "1640998800000000000"
   +      constraintId: 5
          createdExplicitly: true
          foreignKey: {}

diff --git a/pkg/sql/schemachanger/testdata/end_to_end/create_index b/pkg/sql/schemachanger/testdata/end_to_end/create_index
@@ -24,6 +24,7 @@ upsert descriptor #106
   +  mutations:
   +  - direction: ADD
   +    index:
+  +      createdAtNanos: "1640998800000000000"
   +      createdExplicitly: true
   +      foreignKey: {}
   +      geoConfig: {}
@@ -239,7 +240,8 @@ upsert descriptor #106
   -  modificationTime:
   -    wallTime: "1640995200000000008"
   +  indexes:
-  +  - createdExplicitly: true
+  +  - createdAtNanos: "1640998800000000000"
+  +    createdExplicitly: true
   +    foreignKey: {}
   +    geoConfig: {}
   +    id: 2
@@ -262,6 +264,7 @@ upsert descriptor #106
   -  - direction: ADD
   +  - direction: DROP
        index:
+  -      createdAtNanos: "1640998800000000000"
          createdExplicitly: true
          foreignKey: {}
          geoConfig: {}

diff --git a/pkg/sql/schemachanger/testdata/end_to_end/drop_column_create_index_separate_statements b/pkg/sql/schemachanger/testdata/end_to_end/drop_column_create_index_separate_statements
@@ -449,6 +449,7 @@ upsert descriptor #104
   +  - direction: ADD
   +    index:
   +      constraintId: 4
+  +      createdAtNanos: "1640998800000000000"
   +      createdExplicitly: true
   +      foreignKey: {}
   +      geoConfig: {}
@@ -662,6 +663,7 @@ upsert descriptor #104
   -    wallTime: "1640995200000000016"
   +  indexes:
   +  - constraintId: 4
+  +    createdAtNanos: "1640998800000000000"
   +    createdExplicitly: true
   +    foreignKey: {}
   +    geoConfig: {}
@@ -723,7 +725,8 @@ upsert descriptor #104
   +  - direction: DROP
        index:
          constraintId: 4
-  ...
+  -      createdAtNanos: "1640998800000000000"
+         createdExplicitly: true
          foreignKey: {}
          geoConfig: {}
   -      id: 5