Skip to content

Commit

Permalink
sql: allow ALTER DATABASE PRIMARY REGION to work on system tenants
Browse files Browse the repository at this point in the history
Previously, on a multi-region setup the system database could not be modified to be multi-region and it was blocked from being made multi-region aware. To address this, we are now allowing ALTER DATABASE
PRIMARY REGION to work on system tenants.

Fixes: #63365
Epic: CRDB-33032

Release note (sql change): Previously, we added support for settings reegion on the system database, which was limited to tenants only. We lifted this limitation to allow ALTER DATABASE PRIMARY REGION to work on system tenants.
To support preview status, we created a cluster setting called sql.multiregion.preview_multiregion_system_database that will give users the option to set up their system database as multi-region for Cockroach dedicated
(this cluster setting is not enabled by default). Note that after adding non-primary regions, we recommend that users do a rolling restart to propogate region information.
  • Loading branch information
jasminejsun committed Apr 3, 2024
1 parent 49173f4 commit cd7939e
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 4 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/testdata/logic_test/multi_region
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ ALTER DATABASE system PRIMARY REGION "ap-southeast-2"
# are in the system tenant. Hence the lack of error code checking and the
# regex matching both possible outcomes.
skipif config multiregion-9node-3region-3azs-tenant
statement error user testuser may not modify the system database|modifying the regions of system database is not supported
statement error user testuser may not modify the system database|Modifying the regions of system database is not supported. Set up your system database as multi-region using the cluster setting `sql.multiregion.preview_multiregion_system_database.enabled` https://www.cockroachlabs.com/docs/stable/cluster-settings
ALTER DATABASE system PRIMARY REGION "ap-southeast-2"

user root
Expand Down
11 changes: 10 additions & 1 deletion pkg/ccl/multiregionccl/multiregion_system_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestMrSystemDatabase(t *testing.T) {
return cs
}

cluster, _, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(t, 3,
cluster, sqlDB, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(t, 3,
base.TestingKnobs{},
multiregionccltestutils.WithSettings(makeSettings()))
defer cleanup()
Expand Down Expand Up @@ -81,6 +81,15 @@ func TestMrSystemDatabase(t *testing.T) {
// okay.
tDB.CheckQueryResults(t, `SELECT * FROM crdb_internal.invalid_objects`, [][]string{})

_, err = sqlDB.Exec(`SET CLUSTER SETTING sql.multiregion.preview_multiregion_system_database.enabled = true`)
require.NoError(t, err)
_, err = sqlDB.Exec(`ALTER DATABASE system SET PRIMARY REGION "us-east1"`)
require.NoError(t, err)
_, err = sqlDB.Exec(`ALTER DATABASE system ADD REGION "us-east2"`)
require.NoError(t, err)
_, err = sqlDB.Exec(`ALTER DATABASE system ADD REGION "us-east3"`)
require.NoError(t, err)

t.Run("Sqlliveness", func(t *testing.T) {
row := tDB.QueryRow(t, `SELECT crdb_region, session_id, expiration FROM system.sqlliveness LIMIT 1`)
var sessionID string
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/roachtest/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ go_library(
"mixed_version_job_compatibility_in_declarative_schema_changer.go",
"mixed_version_multi_region.go",
"mixed_version_schemachange.go",
"multi_region_system_database.go",
"multiregion_leasing.go",
"multitenant.go",
"multitenant_distsql.go",
Expand Down
127 changes: 127 additions & 0 deletions pkg/cmd/roachtest/tests/multi_region_system_database.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
// Copyright 2024 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package tests

import (
"context"
"fmt"
"strings"
"time"

"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
"github.com/stretchr/testify/require"
)

func registerMultiRegionSystemDatabase(r registry.Registry) {
clusterSpec := r.MakeClusterSpec(3, spec.Geo(), spec.GatherCores(), spec.GCEZones("us-east1-b,us-west1-b,us-central1-b"))
r.Add(registry.TestSpec{
Name: "multi-region-system-database",
Owner: registry.OwnerSQLFoundations,
Timeout: time.Hour * 1,
RequiresLicense: true,
Cluster: clusterSpec,
CompatibleClouds: registry.OnlyGCE,
Suites: registry.Suites(registry.Weekly),
Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
nodes := c.Spec().NodeCount
regions := strings.Split(c.Spec().GCE.Zones, ",")
regionOnly := func(regionAndZone string) string {
r := strings.Split(regionAndZone, "-")
return r[0] + "-" + r[1]
}
t.Status("starting cluster")
c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(install.SecureOption(false)))
conn := c.Conn(ctx, t.L(), 1)
defer conn.Close()

_, err := conn.ExecContext(ctx, "SET CLUSTER SETTING sql.multiregion.preview_multiregion_system_database.enabled = true")
require.NoError(t, err)

_, err = conn.ExecContext(ctx,
fmt.Sprintf(`ALTER DATABASE system SET PRIMARY REGION '%s'`, regionOnly(regions[0])))
require.NoError(t, err)

_, err = conn.ExecContext(ctx,
fmt.Sprintf(`ALTER DATABASE system ADD REGION '%s'`, regionOnly(regions[1])))
require.NoError(t, err)

_, err = conn.ExecContext(ctx,
fmt.Sprintf(`ALTER DATABASE system ADD REGION '%s'`, regionOnly(regions[2])))
require.NoError(t, err)

//Perform rolling restart to propagate region information to non-primary nodes
for i := 2; i <= nodes; i++ {
t.WorkerStatus("stop")
err := c.StopCockroachGracefullyOnNode(ctx, t.L(), i)
if err != nil {
return
}
t.WorkerStatus("start")
startOpts := option.DefaultStartOpts()
c.Start(ctx, t.L(), startOpts, install.MakeClusterSettings(install.SecureOption(false)), c.Node(i))
}

//Check system.lease table to ensure that region information for each node is correct
rows, err := conn.Query("SELECT DISTINCT sql_instance_id, crdb_region FROM system.lease")
require.NoError(t, err)

nodeToRegionName := make(map[int]string)
for rows.Next() {
var sqlInstanceID int
var crdbRegion string
require.NoError(t, rows.Scan(&sqlInstanceID, &crdbRegion))
nodeToRegionName[sqlInstanceID] = crdbRegion
}

for node, regionName := range nodeToRegionName {
require.Equal(t, regionOnly(regions[node-1]), regionName)
}

//Intentionally tear down nodes and ensure that everything is still working
chaosTest := func() {
//Random operations on user-created table
_, err := conn.Exec(`CREATE TABLE foo (key INT PRIMARY KEY)`)
if err != nil {
return
}
defer func() {
_, err := conn.Exec(`DROP TABLE foo`)
if err != nil {
return
}
}()
_, err = conn.Exec(`INSERT INTO foo VALUES (1), (2), (3)`)
require.NoError(t, err)
row := conn.QueryRow(`SELECT * FROM foo LIMIT 1`)
var rowPK int
require.NoError(t, row.Scan(&rowPK))
require.Equal(t, 1, rowPK)
}

for i := 2; i <= nodes; i++ {
t.WorkerStatus("stop")
c.Run(ctx, option.WithNodes(c.Node(i)), "killall -9 cockroach")

t.WorkerStatus("chaos testing")
chaosTest()

t.WorkerStatus("start")
startOpts := option.DefaultStartOpts()
c.Start(ctx, t.L(), startOpts, install.MakeClusterSettings(install.SecureOption(false)), c.Node(i))
}
},
})
}
1 change: 1 addition & 0 deletions pkg/cmd/roachtest/tests/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,5 @@ func RegisterTests(r registry.Registry) {
registerYCSB(r)
registerDeclarativeSchemaChangerJobCompatibilityInMixedVersion(r)
registerMultiRegionMixedVersion(r)
registerMultiRegionSystemDatabase(r)
}
13 changes: 11 additions & 2 deletions pkg/sql/alter_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,11 @@ func (n *alterDatabaseAddRegionNode) startExec(params runParams) error {
if err := params.p.setSystemDatabaseSurvival(params.ctx); err != nil {
return err
}

params.p.BufferClientNotice(
params.ctx,
pgnotice.Newf("Rolling restart is recommended after adding a region to system database in order to propogate region information."),
)
}

// Validate the type descriptor after the changes. We have to do this explicitly here, because
Expand Down Expand Up @@ -586,10 +591,14 @@ func (p *planner) checkPrivilegesForMultiRegionOp(
// multi-region primitives in the system database. For now, we also allow
// root to perform the various operations to enable testing.
if desc.GetID() == keys.SystemDatabaseID {
if p.execCfg.Codec.ForSystemTenant() {
if multiRegionSystemDatabase := sqlclustersettings.MultiRegionSystemDatabaseEnabled.Get(&p.execCfg.Settings.SV); !multiRegionSystemDatabase &&
p.execCfg.Codec.ForSystemTenant() {
return pgerror.Newf(
pgcode.FeatureNotSupported,
"modifying the regions of system database is not supported",
"Modifying the regions of system database is not supported. "+
"Set up your system database as multi-region using the cluster setting "+
"`sql.multiregion.preview_multiregion_system_database.enabled` "+
"https://www.cockroachlabs.com/docs/stable/cluster-settings.",
)
}
if u := p.SessionData().User(); !u.IsNodeUser() && !u.IsRootUser() {
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/sqlclustersettings/clustersettings.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ var SecondaryTenantsAllZoneConfigsEnabled = settings.RegisterBoolSetting(
true,
)

// MultiRegionSystemDatabaseEnabled controls if system tenants are allowed
// to be set up to be multi-region.
var MultiRegionSystemDatabaseEnabled = settings.RegisterBoolSetting(
settings.SystemVisible,
"sql.multiregion.preview_multiregion_system_database.enabled",
"enable option to set up system database as multi-region",
false,
)

// RequireSystemTenantOrClusterSetting returns a setting disabled error if
// executed from inside a secondary tenant that does not have the specified
// cluster setting.
Expand Down

0 comments on commit cd7939e

Please sign in to comment.