Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: remove default_target_cluster.check_service.enabled #120080

Merged
merged 1 commit into from
Mar 12, 2024
Merged

sql: remove default_target_cluster.check_service.enabled #120080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 0 additions & 11 deletions pkg/multitenant/tenant_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 12 additions & 11 deletions pkg/settings/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 0 additions & 21 deletions pkg/sql/logictest/testdata/logic_test/tenant
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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

Expand Down
13 changes: 0 additions & 13 deletions pkg/sql/set_cluster_setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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":
Expand Down
18 changes: 0 additions & 18 deletions pkg/sql/tenant_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down