From ab281232044d89e28c05e18e27a3dfde4adf7b3f Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Tue, 4 Jul 2023 19:23:08 +0200 Subject: [PATCH] sql: session var `disable_drop_virtual_cluster` 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 --- pkg/configprofiles/profiles.go | 4 ++-- pkg/configprofiles/testdata/virtual-app | 4 ++-- pkg/configprofiles/testdata/virtual-app-repl | 4 ++-- pkg/configprofiles/testdata/virtual-noapp | 4 ++-- pkg/configprofiles/testdata/virtual-noapp-repl | 4 ++-- pkg/settings/registry.go | 1 + pkg/sql/exec_util.go | 14 +++++++------- .../testdata/logic_test/information_schema | 2 +- pkg/sql/logictest/testdata/logic_test/tenant | 6 +++--- .../sessiondatapb/local_only_session_data.proto | 6 +++--- pkg/sql/tenant_deletion.go | 4 ++-- pkg/sql/vars.go | 10 +++++----- 12 files changed, 32 insertions(+), 31 deletions(-) diff --git a/pkg/configprofiles/profiles.go b/pkg/configprofiles/profiles.go index daa665b4a939..4ba768432343 100644 --- 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 index c788694786d3..b4b09dcf2c76 100644 --- 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 index c1b8e3d9a160..1bde64d99dae 100644 --- 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 index 2a8c8f21d351..876bb81d5abc 100644 --- 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 index 2efe13c0eb8f..5101a82351b8 100644 --- 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 index 5af4a3a9f776..f5586bdffba1 100644 --- 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 index 2775e11dcecd..388861fa2fd5 100644 --- 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 index 2db5411f8a08..15f0558b8936 100644 --- 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 index e53e884016c2..cfaf565ea760 100644 --- 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 index 6299b9adba06..ba35b92714ea 100644 --- 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 index a553c635ef23..3ff2e49b0b34 100644 --- 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 index 31fb47525613..20c0b17d7797 100644 --- 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)) }, },