sql: remove default_target_cluster.check_service.enabled

Release note (enterprise change): default_target_cluster can now be set to any tenant name by default, including a tenant yet to be created or have service started. Epic: none.
cockroachdb · Mar 12, 2024 · 26acc25 · 26acc25
1 parent 41a474f
commit 26acc25
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 74 deletions.
diff --git a/pkg/multitenant/tenant_config.go b/pkg/multitenant/tenant_config.go
@@ -32,17 +32,6 @@ var DefaultTenantSelect = settings.RegisterStringSetting(
 	settings.WithName(DefaultClusterSelectSettingName),
 )
 
-// VerifyTenantService determines whether there should be an advisory
-// interlock between changes to the tenant service and changes to the
-// above cluster setting.
-var VerifyTenantService = settings.RegisterBoolSetting(
-	settings.SystemOnly,
-	"server.controller.default_tenant.check_service.enabled",
-	"verify that the service mode is coherently set with the value of "+DefaultClusterSelectSettingName,
-	true,
-	settings.WithName(DefaultClusterSelectSettingName+".check_service.enabled"),
-)
-
 // WaitForClusterStartTimeout is the amount of time the tenant
 // controller will wait for the default virtual cluster to have an
 // active SQL server.

diff --git a/pkg/settings/registry.go b/pkg/settings/registry.go
@@ -229,17 +229,18 @@ var retiredSettings = map[InternalKey]struct{}{
 	"bulkio.restore.remove_regions.enabled":                    {},
 
 	// removed as of 24.1
-	"storage.mvcc.range_tombstones.enabled":                {},
-	"changefeed.balance_range_distribution.enable":         {},
-	"changefeed.mux_rangefeed.enabled":                     {},
-	"kv.rangefeed.catchup_scan_concurrency":                {},
-	"kv.rangefeed.scheduler.enabled":                       {},
-	"physical_replication.producer.mux_rangefeeds.enabled": {},
-	"kv.rangefeed.use_dedicated_connection_class.enabled":  {},
-	"sql.trace.session_eventlog.enabled":                   {},
-	"sql.show_ranges_deprecated_behavior.enabled":          {},
-	"sql.drop_virtual_cluster.enabled":                     {},
-	"cross_cluster_replication.enabled":                    {},
+	"storage.mvcc.range_tombstones.enabled":                  {},
+	"changefeed.balance_range_distribution.enable":           {},
+	"changefeed.mux_rangefeed.enabled":                       {},
+	"kv.rangefeed.catchup_scan_concurrency":                  {},
+	"kv.rangefeed.scheduler.enabled":                         {},
+	"physical_replication.producer.mux_rangefeeds.enabled":   {},
+	"kv.rangefeed.use_dedicated_connection_class.enabled":    {},
+	"sql.trace.session_eventlog.enabled":                     {},
+	"sql.show_ranges_deprecated_behavior.enabled":            {},
+	"sql.drop_virtual_cluster.enabled":                       {},
+	"cross_cluster_replication.enabled":                      {},
+	"server.controller.default_tenant.check_service.enabled": {},
 }
 
 // sqlDefaultSettings is the list of "grandfathered" existing sql.defaults

diff --git a/pkg/sql/logictest/testdata/logic_test/tenant b/pkg/sql/logictest/testdata/logic_test/tenant
@@ -514,21 +514,9 @@ subtest regression_105115
 statement ok
 CREATE TENANT noservice
 
-statement error shared service not enabled for tenant "noservice"
-SET CLUSTER SETTING server.controller.default_target_cluster = noservice
-
-statement ok
-SET CLUSTER SETTING server.controller.default_target_cluster.check_service.enabled = false
-
 statement ok
 SET CLUSTER SETTING server.controller.default_target_cluster = noservice
 
-statement ok
-RESET CLUSTER SETTING server.controller.default_target_cluster.check_service.enabled
-
-statement ok
-RESET CLUSTER SETTING server.controller.default_target_cluster
-
 statement ok
 DROP TENANT noservice;
 CREATE TENANT withservice;
@@ -539,19 +527,10 @@ ALTER TENANT withservice START SERVICE SHARED
 statement ok
 SET CLUSTER SETTING server.controller.default_target_cluster = withservice
 
-statement error cannot stop service while tenant is selected as default
-ALTER TENANT withservice STOP SERVICE
-
-statement ok
-SET CLUSTER SETTING server.controller.default_target_cluster.check_service.enabled = false
-
 statement ok
 ALTER TENANT withservice STOP SERVICE
 
 # clean up
-statement ok
-RESET CLUSTER SETTING server.controller.default_target_cluster.check_service.enabled
-
 statement ok
 RESET CLUSTER SETTING server.controller.default_target_cluster
 

diff --git a/pkg/sql/set_cluster_setting.go b/pkg/sql/set_cluster_setting.go
@@ -22,8 +22,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/docs"
 	"github.com/cockroachdb/cockroach/pkg/kv"
-	"github.com/cockroachdb/cockroach/pkg/multitenant"
-	"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/server/settingswatcher"
@@ -358,17 +356,6 @@ func (n *setClusterSettingNode) startExec(params runParams) error {
 	// Report tracked cluster settings via telemetry.
 	// TODO(justin): implement a more general mechanism for tracking these.
 	switch n.name {
-	case multitenant.DefaultClusterSelectSettingName:
-		if multitenant.VerifyTenantService.Get(&n.st.SV) && expectedEncodedValue != "" {
-			tr, err := GetTenantRecordByName(params.ctx, n.st, params.p.InternalSQLTxn(), roachpb.TenantName(expectedEncodedValue))
-			if err != nil {
-				return errors.Wrapf(err, "failed to lookup tenant %q", expectedEncodedValue)
-			}
-			if tr.ServiceMode != mtinfopb.ServiceModeShared {
-				return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
-					"shared service not enabled for tenant %q", expectedEncodedValue)
-			}
-		}
 	case catpb.AutoStatsEnabledSettingName:
 		switch expectedEncodedValue {
 		case "true":

diff --git a/pkg/sql/tenant_update.go b/pkg/sql/tenant_update.go
@@ -16,7 +16,6 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
-	"github.com/cockroachdb/cockroach/pkg/multitenant"
 	"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
@@ -110,23 +109,6 @@ func validateTenantInfo(
 			info.ServiceMode, info.DataState)
 	}
 
-	// Sanity check. Note that this interlock is not a guarantee that
-	// the cluster setting will never be set to an invalid tenant. There
-	// is a race condition between changing the cluster setting and the
-	// check here. Generally, other subsystems should always tolerate
-	// when the cluster setting is set to a tenant without service (or
-	// even one that doesn't exist).
-	if multitenant.VerifyTenantService.Get(&settings.SV) &&
-		info.ServiceMode == mtinfopb.ServiceModeNone &&
-		info.Name != "" &&
-		multitenant.DefaultTenantSelect.Get(&settings.SV) == string(info.Name) {
-		return errors.WithHintf(
-			pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
-				"cannot stop service while tenant is selected as default"),
-			"Update the cluster setting %q to a different value.",
-			multitenant.DefaultClusterSelectSettingName)
-	}
-
 	return nil
 }