sql: allow ALTER DATABASE PRIMARY REGION to work on system tenants

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.

pkg/ccl/logictestccl/testdata/logic_test/multi_region

@@ -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

pkg/ccl/multiregionccl/multiregion_system_table_test.go

@@ -307,6 +307,93 @@ func TestMrSystemDatabase(t *testing.T) {
 	})
 }
 
+func TestMRSystemDatabase(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	_, sqlDB, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(
+		t, 3, base.TestingKnobs{},
+	)
+	defer cleanup()
+
+	_, 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 := sqlDB.QueryRow(`SELECT crdb_region, session_id, expiration FROM system.sqlliveness LIMIT 1`)
+		var sessionID string
+		var crdbRegion string
+		var rawExpiration apd.Decimal
+		require.NoError(t, row.Scan(&crdbRegion, &sessionID, &rawExpiration))
+		require.Equal(t, "us-east1", crdbRegion)
+	})
+
+	t.Run("Sqlinstances", func(t *testing.T) {
+		t.Run("InUse", func(t *testing.T) {
+			query := `
+                SELECT id, addr, session_id, locality, crdb_region
+                FROM system.sql_instances
+                WHERE session_id IS NOT NULL
+            `
+			rows, _ := sqlDB.Query(query)
+			require.True(t, rows.Next())
+			for {
+				var id base.SQLInstanceID
+				var addr, locality string
+				var crdb_region string
+				var session sqlliveness.SessionID
+
+				require.NoError(t, rows.Scan(&id, &addr, &session, &locality, &crdb_region))
+
+				require.True(t, 0 < id)
+				require.NotEmpty(t, addr)
+				require.NotEmpty(t, locality)
+				require.NotEmpty(t, session)
+				require.NotEmpty(t, crdb_region)
+
+				require.Equal(t, "us-east1", crdb_region)
+
+				if !rows.Next() {
+					break
+				}
+			}
+			require.NoError(t, rows.Close())
+		})
+	})
+
+	t.Run("Reclaim", func(t *testing.T) {
+		id := uuid.MakeV4()
+		s1, err := slstorage.MakeSessionID(make([]byte, 100), id)
+		require.NoError(t, err)
+		s2, err := slstorage.MakeSessionID(make([]byte, 200), id)
+		require.NoError(t, err)
+
+		// Insert expired entries into sql_instances.
+		_, err = sqlDB.Exec(`INSERT INTO system.sql_instances (id, addr, session_id, locality, crdb_region) VALUES
+		   		(100, NULL, $1, NULL, 'us-east2'),
+		   		(200, NULL, $2, NULL, 'us-east3')`, s1.UnsafeBytes(), s2.UnsafeBytes())
+		require.NoError(t, err)
+
+		query := `SELECT count(*) FROM system.sql_instances WHERE id = 42`
+
+		// Wait until expired entries get removed.
+		testutils.SucceedsSoon(t, func() error {
+			var rowCount int
+			require.NoError(t, sqlDB.QueryRow(query).Scan(&rowCount))
+			if rowCount != 0 {
+				return errors.New("some regions have not been reclaimed")
+			}
+			return nil
+		})
+	})
+}
+
 // TestMultiRegionTenantRegions tests the behavior of region-related
 // commands in the context of a multi-region tenant (a tenant with a
 // multi-region system database).

pkg/cmd/roachtest/tests/BUILD.bazel

@@ -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",

pkg/cmd/roachtest/tests/multi_region_system_database.go

@@ -0,0 +1,128 @@
+// 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")
+				stopOpts := option.DefaultStopOpts()
+				c.Stop(ctx, t.L(), stopOpts, c.Node(i))
+
+				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))
+			}
+		},
+	})
+}

pkg/cmd/roachtest/tests/registry.go

@@ -154,4 +154,5 @@ func RegisterTests(r registry.Registry) {
 	registerYCSB(r)
 	registerDeclarativeSchemaChangerJobCompatibilityInMixedVersion(r)
 	registerMultiRegionMixedVersion(r)
+	registerMultiRegionSystemDatabase(r)
 }

pkg/sql/alter_database.go

@@ -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
@@ -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() {

pkg/sql/sqlclustersettings/clustersettings.go

@@ -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.