Merge cockroachdb#106121

106121: sql: session var `disable_drop_virtual_cluster` r=yuzefovich a=knz Informs cockroachdb#106068. Epic: CRDB-29380 This replaces `disable_drop_tenant`, together with the cluster setting `sql.drop_virtual_cluster.enabled` (replaces `sql.drop_tenant.enabled`). We are OK with this breaking change since the feature was not yet exposed to end-users. Release note: None Co-authored-by: Raphael 'kena' Poss <[email protected]>
AlexTalks · Jul 6, 2023 · 06fde61 · 06fde61
2 parents 818aec8 + ab28123
commit 06fde61
Show file tree

Hide file tree

Showing 12 changed files with 32 additions and 31 deletions.
diff --git a/pkg/configprofiles/profiles.go b/pkg/configprofiles/profiles.go
@@ -92,8 +92,8 @@ var virtClusterInitTasks = []autoconfigpb.Task{
 			// to range coalescing have not been solved yet).
 			"SET CLUSTER SETTING spanconfig.storage_coalesce_adjacent.enabled = false",
 			"SET CLUSTER SETTING spanconfig.tenant_coalesce_adjacent.enabled = false",
-			// Make the operator double-check tenant deletions.
-			"SET CLUSTER SETTING sql.drop_tenant.enabled = false",
+			// Make the operator double-check virtual cluster deletions.
+			"SET CLUSTER SETTING sql.drop_virtual_cluster.enabled = false",
 		},
 		nil, /* txnSQL */
 	),

diff --git a/pkg/configprofiles/testdata/virtual-app b/pkg/configprofiles/testdata/virtual-app
@@ -18,7 +18,7 @@ WHERE variable IN (
 'sql.multi_region.allow_abstractions_for_secondary_tenants.enabled',
 'spanconfig.storage_coalesce_adjacent.enabled',
 'spanconfig.tenant_coalesce_adjacent.enabled',
-'sql.drop_tenant.enabled',
+'sql.drop_virtual_cluster.enabled',
 'sql.create_tenant.default_template',
 'server.controller.default_tenant',
 'kv.rangefeed.enabled',
@@ -33,7 +33,7 @@ server.secondary_tenants.redact_trace.enabled false
 spanconfig.storage_coalesce_adjacent.enabled false
 spanconfig.tenant_coalesce_adjacent.enabled false
 sql.create_tenant.default_template template
-sql.drop_tenant.enabled false
+sql.drop_virtual_cluster.enabled false
 sql.multi_region.allow_abstractions_for_secondary_tenants.enabled true
 sql.zone_configs.allow_for_secondary_tenant.enabled true
 

diff --git a/pkg/configprofiles/testdata/virtual-app-repl b/pkg/configprofiles/testdata/virtual-app-repl
@@ -18,7 +18,7 @@ WHERE variable IN (
 'sql.multi_region.allow_abstractions_for_secondary_tenants.enabled',
 'spanconfig.storage_coalesce_adjacent.enabled',
 'spanconfig.tenant_coalesce_adjacent.enabled',
-'sql.drop_tenant.enabled',
+'sql.drop_virtual_cluster.enabled',
 'sql.create_tenant.default_template',
 'server.controller.default_tenant',
 'kv.rangefeed.enabled',
@@ -33,7 +33,7 @@ server.secondary_tenants.redact_trace.enabled false
 spanconfig.storage_coalesce_adjacent.enabled false
 spanconfig.tenant_coalesce_adjacent.enabled false
 sql.create_tenant.default_template template
-sql.drop_tenant.enabled false
+sql.drop_virtual_cluster.enabled false
 sql.multi_region.allow_abstractions_for_secondary_tenants.enabled true
 sql.zone_configs.allow_for_secondary_tenant.enabled true
 

diff --git a/pkg/configprofiles/testdata/virtual-noapp b/pkg/configprofiles/testdata/virtual-noapp
@@ -13,7 +13,7 @@ WHERE variable IN (
 'sql.multi_region.allow_abstractions_for_secondary_tenants.enabled',
 'spanconfig.storage_coalesce_adjacent.enabled',
 'spanconfig.tenant_coalesce_adjacent.enabled',
-'sql.drop_tenant.enabled',
+'sql.drop_virtual_cluster.enabled',
 'sql.create_tenant.default_template',
 'server.controller.default_tenant',
 'kv.rangefeed.enabled',
@@ -28,7 +28,7 @@ server.secondary_tenants.redact_trace.enabled false
 spanconfig.storage_coalesce_adjacent.enabled false
 spanconfig.tenant_coalesce_adjacent.enabled false
 sql.create_tenant.default_template template
-sql.drop_tenant.enabled false
+sql.drop_virtual_cluster.enabled false
 sql.multi_region.allow_abstractions_for_secondary_tenants.enabled true
 sql.zone_configs.allow_for_secondary_tenant.enabled true
 

diff --git a/pkg/configprofiles/testdata/virtual-noapp-repl b/pkg/configprofiles/testdata/virtual-noapp-repl
@@ -13,7 +13,7 @@ WHERE variable IN (
 'sql.multi_region.allow_abstractions_for_secondary_tenants.enabled',
 'spanconfig.storage_coalesce_adjacent.enabled',
 'spanconfig.tenant_coalesce_adjacent.enabled',
-'sql.drop_tenant.enabled',
+'sql.drop_virtual_cluster.enabled',
 'sql.create_tenant.default_template',
 'server.controller.default_tenant',
 'kv.rangefeed.enabled',
@@ -28,7 +28,7 @@ server.secondary_tenants.redact_trace.enabled false
 spanconfig.storage_coalesce_adjacent.enabled false
 spanconfig.tenant_coalesce_adjacent.enabled false
 sql.create_tenant.default_template template
-sql.drop_tenant.enabled false
+sql.drop_virtual_cluster.enabled false
 sql.multi_region.allow_abstractions_for_secondary_tenants.enabled true
 sql.zone_configs.allow_for_secondary_tenant.enabled true
 

diff --git a/pkg/settings/registry.go b/pkg/settings/registry.go
@@ -164,6 +164,7 @@ var retiredSettings = map[string]struct{}{
 	// renamed.
 	"spanconfig.host_coalesce_adjacent.enabled":            {},
 	"sql.defaults.experimental_stream_replication.enabled": {},
+	"sql.drop_tenant.enabled":                              {},
 
 	// removed as of 23.2.
 	"sql.log.unstructured_entries.enabled":                     {},

diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go
@@ -744,8 +744,8 @@ var errTransactionInProgress = errors.New("there is already a transaction in pro
 const sqlTxnName string = "sql txn"
 const metricsSampleInterval = 10 * time.Second
 
-// enableDropTenant (or rather, its inverted boolean value) defines
-// the default value for the session var "disable_drop_tenant".
+// enableDropVirtualCluster (or rather, its inverted boolean value) defines
+// the default value for the session var "disable_drop_virtual_cluster".
 //
 // Note:
 //   - We use a cluster setting here instead of a default role option
@@ -756,10 +756,10 @@ const metricsSampleInterval = 10 * time.Second
 //   - The session var is named "disable_" because we want the Go
 //     default value (false) to mean that tenant deletion is enabled.
 //     This is needed for backward-compatibility with Cockroach Cloud.
-var enableDropTenant = settings.RegisterBoolSetting(
+var enableDropVirtualCluster = settings.RegisterBoolSetting(
 	settings.SystemOnly,
-	"sql.drop_tenant.enabled",
-	"default value (inverted) for the disable_drop_tenant session setting",
+	"sql.drop_virtual_cluster.enabled",
+	"default value (inverted) for the disable_virtual_cluster session setting",
 	true,
 )
 
@@ -3226,8 +3226,8 @@ func (m *sessionDataMutator) SetSafeUpdates(val bool) {
 	m.data.SafeUpdates = val
 }
 
-func (m *sessionDataMutator) SetDisableDropTenant(val bool) {
-	m.data.DisableDropTenant = val
+func (m *sessionDataMutator) SetDisableDropVirtualCluster(val bool) {
+	m.data.DisableDropVirtualCluster = val
 }
 
 func (m *sessionDataMutator) SetCheckFunctionBodies(val bool) {

diff --git a/pkg/sql/logictest/testdata/logic_test/information_schema b/pkg/sql/logictest/testdata/logic_test/information_schema
@@ -5275,7 +5275,7 @@ default_transaction_read_only                              off
 default_transaction_use_follower_reads                     off
 default_with_oids                                          off
 descriptor_validation                                      on
-disable_drop_tenant                                        off
+disable_drop_virtual_cluster                               off
 disable_hoist_projection_in_join_limitation                off
 disable_partially_distributed_plans                        off
 disable_plan_gists                                         off

diff --git a/pkg/sql/logictest/testdata/logic_test/tenant b/pkg/sql/logictest/testdata/logic_test/tenant
@@ -487,10 +487,10 @@ DROP TENANT tmpl
 statement ok
 RESET CLUSTER SETTING sql.create_tenant.default_template
 
-subtest block_drop_tenant
+subtest block_drop_virtual_cluster
 
 statement ok
-SET disable_drop_tenant = 'true'
+SET disable_drop_virtual_cluster = 'true'
 
 statement ok
 CREATE TENANT nodelete
@@ -499,7 +499,7 @@ statement error rejected.*irreversible data loss
 DROP TENANT nodelete
 
 statement ok
-RESET disable_drop_tenant
+RESET disable_drop_virtual_cluster
 
 statement ok
 DROP TENANT nodelete

diff --git a/pkg/sql/sessiondatapb/local_only_session_data.proto b/pkg/sql/sessiondatapb/local_only_session_data.proto
@@ -369,9 +369,9 @@ message LocalOnlySessionData {
   int64 prepared_statements_cache_size = 97;
   // StreamerEnabled controls whether the Streamer API can be used.
   bool streamer_enabled = 98;
-  // DisableDropTenant causes errors when the client
-  // attempts to drop tenants or tenant records.
-  bool disable_drop_tenant = 99;
+  // DisableDropVirtualCluster causes errors when the client
+  // attempts to drop virtual clusters or tenant records.
+  bool disable_drop_virtual_cluster = 99;
   // MultipleActivePortalEnabled determines if the pgwire portal execution
   // for certain queries can be paused. If true, portals with read-only SELECT
   // query without sub/post queries can be executed in interleaving manner, but

diff --git a/pkg/sql/tenant_deletion.go b/pkg/sql/tenant_deletion.go
@@ -32,9 +32,9 @@ import (
 func (p *planner) DropTenantByID(
 	ctx context.Context, tenID uint64, synchronousImmediateDrop, ignoreServiceMode bool,
 ) error {
-	if p.SessionData().DisableDropTenant || p.SessionData().SafeUpdates {
+	if p.SessionData().DisableDropVirtualCluster || p.SessionData().SafeUpdates {
 		err := errors.Newf("DROP VIRTUAL CLUSTER causes irreversible data loss")
-		err = errors.WithMessage(err, "rejected (via sql_safe_updates or disable_drop_tenant)")
+		err = errors.WithMessage(err, "rejected (via sql_safe_updates or disable_drop_virtual_cluster)")
 		err = pgerror.WithCandidateCode(err, pgcode.Warning)
 		return err
 	}

diff --git a/pkg/sql/vars.go b/pkg/sql/vars.go
@@ -1566,17 +1566,17 @@ var varGen = map[string]sessionVar{
 	},
 
 	// CockroachDB extension.
-	`disable_drop_tenant`: {
+	`disable_drop_virtual_cluster`: {
 		Hidden: true,
 		Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
-			return formatBoolAsPostgresSetting(evalCtx.SessionData().DisableDropTenant), nil
+			return formatBoolAsPostgresSetting(evalCtx.SessionData().DisableDropVirtualCluster), nil
 		},
 		Set: func(_ context.Context, m sessionDataMutator, s string) error {
-			b, err := paramparse.ParseBoolVar("disable_drop_tenant", s)
+			b, err := paramparse.ParseBoolVar("disable_drop_virtual_cluster", s)
 			if err != nil {
 				return err
 			}
-			m.SetDisableDropTenant(b)
+			m.SetDisableDropVirtualCluster(b)
 			return nil
 		},
 		GlobalDefault: func(sv *settings.Values) string {
@@ -1589,7 +1589,7 @@ var varGen = map[string]sessionVar{
 			// - The session var is named "disable_" because we want the Go
 			//   default value (false) to mean that tenant deletion is enabled.
 			//   This is needed for backward-compatibility with Cockroach Cloud.
-			return formatBoolAsPostgresSetting(!enableDropTenant.Get(sv))
+			return formatBoolAsPostgresSetting(!enableDropVirtualCluster.Get(sv))
 		},
 	},