Skip to content

Commit

Permalink
Merge #61073
Browse files Browse the repository at this point in the history
61073: sql: update database zone configurations after region drop finalization r=otan a=arulajmani

Previously, database level zone configurations weren't updated when the
region drop was finalized. This patch adds that support.

This patch also moves setting zone configurations on ADD REGION from
the user txn to the type schema changer. This ensures that zone
configurations are added transactionally -- if the ADD REGION fails for
whatever reason, we no longer leave behind dangling zone config.

Lastly, I've refactored some test setup code that was being duplicated
and improved the tests around rollback to test additional scenarios.

Closes #60435
Closes #60750

Release note: None

Release justification: bug fixes and low-risk updates to new
functionality

Co-authored-by: arulajmani <[email protected]>
  • Loading branch information
craig[bot] and arulajmani committed Feb 26, 2021
2 parents 45deb66 + bd6d789 commit eb0aa06
Show file tree
Hide file tree
Showing 10 changed files with 262 additions and 178 deletions.
32 changes: 32 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region
Original file line number Diff line number Diff line change
Expand Up @@ -1083,3 +1083,35 @@ ALTER TABLE drop_primary_regions_db.primary SET LOCALITY REGIONAL BY TABLE IN PR

statement ok
ALTER DATABASE drop_primary_regions_db DROP REGION "ca-central-1"

statement ok
CREATE DATABASE zone_config_drop_region PRIMARY REGION "ca-central-1" REGIONS "us-east-1", "ap-southeast-2"

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

statement ok
ALTER DATABASE zone_config_drop_region DROP REGION "us-east-1"

query TT
SHOW ZONE CONFIGURATION FOR DATABASE zone_config_drop_region
----
DATABASE zone_config_drop_region ALTER DATABASE zone_config_drop_region 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=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'
24 changes: 12 additions & 12 deletions pkg/ccl/logictestccl/testdata/logic_test/regional_by_row
Original file line number Diff line number Diff line change
Expand Up @@ -1832,9 +1832,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 90000,
num_replicas = 5,
num_replicas = 4,
num_voters = 3,
constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}',
constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1}',
voter_constraints = '[+region=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'

Expand Down Expand Up @@ -1874,9 +1874,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 90000,
num_replicas = 5,
num_replicas = 4,
num_voters = 3,
constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}',
constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1}',
voter_constraints = '[+region=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'

Expand Down Expand Up @@ -1924,9 +1924,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 90000,
num_replicas = 5,
num_replicas = 4,
num_voters = 3,
constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}',
constraints = '{+region=ca-central-1: 1, +region=us-east-1: 1}',
voter_constraints = '[+region=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'

Expand Down Expand Up @@ -1966,9 +1966,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 90000,
num_replicas = 5,
num_replicas = 4,
num_voters = 3,
constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}',
constraints = '{+region=ca-central-1: 1, +region=us-east-1: 1}',
voter_constraints = '[+region=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'

Expand Down Expand Up @@ -2006,9 +2006,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 90000,
num_replicas = 5,
num_replicas = 3,
num_voters = 3,
constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}',
constraints = '{+region=ca-central-1: 1}',
voter_constraints = '[+region=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'

Expand All @@ -2019,9 +2019,9 @@ DATABASE drop_regions ALTER DATABASE drop_regions CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 90000,
num_replicas = 5,
num_replicas = 3,
num_voters = 3,
constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}',
constraints = '{+region=ca-central-1: 1}',
voter_constraints = '[+region=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'

Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/multiregionccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ go_test(
],
deps = [
"//pkg/base",
"//pkg/ccl/multiregionccl/multiregionccltestutils",
"//pkg/ccl/partitionccl",
"//pkg/ccl/utilccl",
"//pkg/jobs",
Expand Down
9 changes: 7 additions & 2 deletions pkg/ccl/multiregionccl/multiregion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/ccl/multiregionccl/multiregionccltestutils"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
Expand All @@ -26,7 +27,9 @@ func TestMultiRegionNoLicense(t *testing.T) {
defer log.Scope(t).Close(t)
defer utilccl.TestingDisableEnterprise()()

_, sqlDB, cleanup := createTestMultiRegionCluster(t, 3, base.TestingKnobs{})
_, sqlDB, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(
t, 3 /* numServers */, base.TestingKnobs{},
)
defer cleanup()

_, err := sqlDB.Exec(`CREATE DATABASE test`)
Expand All @@ -51,7 +54,9 @@ func TestMultiRegionAfterEnterpriseDisabled(t *testing.T) {

skip.UnderRace(t, "#61163")

_, sqlDB, cleanup := createTestMultiRegionCluster(t, 3, base.TestingKnobs{})
_, sqlDB, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(
t, 3 /* numServers */, base.TestingKnobs{},
)
defer cleanup()

_, err := sqlDB.Exec(`
Expand Down
13 changes: 13 additions & 0 deletions pkg/ccl/multiregionccl/multiregionccltestutils/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "multiregionccltestutils",
srcs = ["testutils.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/multiregionccl/multiregionccltestutils",
visibility = ["//visibility:public"],
deps = [
"//pkg/base",
"//pkg/roachpb",
"//pkg/testutils/serverutils",
],
)
56 changes: 56 additions & 0 deletions pkg/ccl/multiregionccl/multiregionccltestutils/testutils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2021 The Cockroach Authors.
//
// Licensed as a CockroachDB Enterprise file under the Cockroach Community
// License (the "License"); you may not use this file except in compliance with
// the License. You may obtain a copy of the License at
//
// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt

package multiregionccltestutils

import (
"context"
gosql "database/sql"
"fmt"
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
)

// TestingCreateMultiRegionCluster creates a test cluster with numServers number
// of nodes and the provided testing knobs applied to each of the nodes. Every
// node is placed in its own locality, named "us-east1", "us-east2", and so on.
func TestingCreateMultiRegionCluster(
t *testing.T, numServers int, knobs base.TestingKnobs,
) (serverutils.TestClusterInterface, *gosql.DB, func()) {
serverArgs := make(map[int]base.TestServerArgs)
regionNames := make([]string, numServers)
for i := 0; i < numServers; i++ {
// "us-east1", "us-east2"...
regionNames[i] = fmt.Sprintf("us-east%d", i+1)
}

for i := 0; i < numServers; i++ {
serverArgs[i] = base.TestServerArgs{
Knobs: knobs,
Locality: roachpb.Locality{
Tiers: []roachpb.Tier{{Key: "region", Value: regionNames[i]}},
},
}
}

tc := serverutils.StartNewTestCluster(t, numServers, base.TestClusterArgs{
ServerArgsPerNode: serverArgs,
})

ctx := context.Background()
cleanup := func() {
tc.Stopper().Stop(ctx)
}

sqlDB := tc.ServerConn(0)

return tc, sqlDB, cleanup
}
Loading

0 comments on commit eb0aa06

Please sign in to comment.