server: remove vestigial tenant-only testing knob

This knob was being used by default to subvert the settings infrastructure in
tenants on the local node. This lead to hazardous interactions with the
settingswatcher behavior. That library tries quite hard to synchronous updates
to settings and ensure that they do not regress. By setting the setting above
that layer, we could very much see them regress.

As far as I can tell, this code came about before tenants could actually manage
settings for themselves. In practice, this code would run prior to the
transaction writing the setting running, which generally meant that so long
as you didn't flip settings back and forth, things would work out.
Nevertheless, it was tech debt and is now removed.

Fixes cockroachdb#87017
Informs cockroachdb#87201

Release note: None

pkg/base/test_server_args.go

@@ -243,10 +243,6 @@ type TestTenantArgs struct {
 	// tenant cluster.
 	Settings *cluster.Settings
 
-	// AllowSettingClusterSettings, if true, allows the tenant to set in-memory
-	// cluster settings.
-	AllowSettingClusterSettings bool
-
 	// Stopper, if not nil, is used to stop the tenant manually otherwise the
 	// TestServer stopper will be used.
 	Stopper *stop.Stopper

pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go

@@ -855,9 +855,8 @@ func TestConsumption(t *testing.T) {
 	testProvider := newTestProvider()
 
 	_, tenantDB := serverutils.StartTenant(t, hostServer, base.TestTenantArgs{
-		TenantID:                    serverutils.TestTenantID(),
-		Settings:                    st,
-		AllowSettingClusterSettings: true,
+		TenantID: serverutils.TestTenantID(),
+		Settings: st,
 		TestingKnobs: base.TestingKnobs{
 			TenantTestingKnobs: &sql.TenantTestingKnobs{
 				OverrideTokenBucketProvider: func(kvtenant.TokenBucketProvider) kvtenant.TokenBucketProvider {
@@ -932,9 +931,8 @@ func TestSQLLivenessExemption(t *testing.T) {
 	slinstance.DefaultHeartBeat.Override(ctx, &st.SV, 10*time.Millisecond)
 
 	_, tenantDB := serverutils.StartTenant(t, hostServer, base.TestTenantArgs{
-		TenantID:                    tenantID,
-		Settings:                    st,
-		AllowSettingClusterSettings: true,
+		TenantID: tenantID,
+		Settings: st,
 	})
 
 	r := sqlutils.MakeSQLRunner(tenantDB)
@@ -994,9 +992,8 @@ func TestScheduledJobsConsumption(t *testing.T) {
 	var tenantServer serverutils.TestTenantInterface
 	var tenantDB *gosql.DB
 	tenantServer, tenantDB = serverutils.StartTenant(t, hostServer, base.TestTenantArgs{
-		TenantID:                    serverutils.TestTenantID(),
-		Settings:                    st,
-		AllowSettingClusterSettings: true,
+		TenantID: serverutils.TestTenantID(),
+		Settings: st,
 		TestingKnobs: base.TestingKnobs{
 			TenantTestingKnobs: &sql.TenantTestingKnobs{
 				OverrideTokenBucketProvider: func(kvtenant.TokenBucketProvider) kvtenant.TokenBucketProvider {
@@ -1081,9 +1078,8 @@ func TestConsumptionChangefeeds(t *testing.T) {
 	testProvider := newTestProvider()
 
 	_, tenantDB := serverutils.StartTenant(t, hostServer, base.TestTenantArgs{
-		TenantID:                    serverutils.TestTenantID(),
-		Settings:                    st,
-		AllowSettingClusterSettings: true,
+		TenantID: serverutils.TestTenantID(),
+		Settings: st,
 		TestingKnobs: base.TestingKnobs{
 			TenantTestingKnobs: &sql.TenantTestingKnobs{
 				OverrideTokenBucketProvider: func(kvtenant.TokenBucketProvider) kvtenant.TokenBucketProvider {
@@ -1153,10 +1149,9 @@ func TestConsumptionExternalStorage(t *testing.T) {
 
 	testProvider := newTestProvider()
 	_, tenantDB := serverutils.StartTenant(t, hostServer, base.TestTenantArgs{
-		TenantID:                    serverutils.TestTenantID(),
-		Settings:                    st,
-		AllowSettingClusterSettings: true,
-		ExternalIODir:               dir,
+		TenantID:      serverutils.TestTenantID(),
+		Settings:      st,
+		ExternalIODir: dir,
 		TestingKnobs: base.TestingKnobs{
 			Server: &server.TestingKnobs{
 				BlobClientFactory: blobClientFactory,
@@ -1260,9 +1255,8 @@ func BenchmarkExternalIOAccounting(b *testing.B) {
 	st := cluster.MakeTestingClusterSettings()
 	tenantcostmodel.ExternalIOEgressCostPerMiB.Override(context.Background(), &st.SV, 0.0)
 	tenantS, _ := serverutils.StartTenant(b, hostServer, base.TestTenantArgs{
-		TenantID:                    serverutils.TestTenantID(),
-		Settings:                    st,
-		AllowSettingClusterSettings: true,
+		TenantID: serverutils.TestTenantID(),
+		Settings: st,
 	})
 
 	nullsink.NullRequiresExternalIOAccounting = true

pkg/ccl/serverccl/diagnosticsccl/reporter_test.go

@@ -53,9 +53,8 @@ func TestTenantReport(t *testing.T) {
 	defer rt.Close()
 
 	tenantArgs := base.TestTenantArgs{
-		TenantID:                    serverutils.TestTenantID(),
-		AllowSettingClusterSettings: true,
-		TestingKnobs:                rt.testingKnobs,
+		TenantID:     serverutils.TestTenantID(),
+		TestingKnobs: rt.testingKnobs,
 	}
 	tenant, tenantDB := serverutils.StartTenant(t, rt.server, tenantArgs)
 	reporter := tenant.DiagnosticsReporter().(*diagnostics.Reporter)

pkg/ccl/serverccl/server_sql_test.go

@@ -82,7 +82,7 @@ func TestTenantCannotSetClusterSetting(t *testing.T) {
 	defer tc.Stopper().Stop(ctx)
 
 	// StartTenant with the default permissions to
-	_, db := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{TenantID: serverutils.TestTenantID(), AllowSettingClusterSettings: false})
+	_, db := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{TenantID: serverutils.TestTenantID()})
 	defer db.Close()
 	_, err := db.Exec(`SET CLUSTER SETTING sql.defaults.vectorize=off`)
 	require.NoError(t, err)
@@ -109,7 +109,7 @@ func TestTenantCanUseEnterpriseFeatures(t *testing.T) {
 	tc := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{})
 	defer tc.Stopper().Stop(context.Background())
 
-	_, db := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{TenantID: serverutils.TestTenantID(), AllowSettingClusterSettings: false})
+	_, db := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{TenantID: serverutils.TestTenantID()})
 	defer db.Close()
 
 	_, err := db.Exec(`BACKUP INTO 'userfile:///backup'`)

pkg/server/testserver.go

@@ -533,17 +533,16 @@ func (ts *TestServer) maybeStartDefaultTestTenant(ctx context.Context) error {
 	params := base.TestTenantArgs{
 		// Currently, all the servers leverage the same tenant ID. We may
 		// want to change this down the road, for more elaborate testing.
-		TenantID:                    serverutils.TestTenantID(),
-		MemoryPoolSize:              ts.params.SQLMemoryPoolSize,
-		TempStorageConfig:           &tempStorageConfig,
-		Locality:                    ts.params.Locality,
-		ExternalIODir:               ts.params.ExternalIODir,
-		ExternalIODirConfig:         ts.params.ExternalIODirConfig,
-		ForceInsecure:               ts.Insecure(),
-		UseDatabase:                 ts.params.UseDatabase,
-		SSLCertsDir:                 ts.params.SSLCertsDir,
-		AllowSettingClusterSettings: true,
-		TestingKnobs:                ts.params.Knobs,
+		TenantID:            serverutils.TestTenantID(),
+		MemoryPoolSize:      ts.params.SQLMemoryPoolSize,
+		TempStorageConfig:   &tempStorageConfig,
+		Locality:            ts.params.Locality,
+		ExternalIODir:       ts.params.ExternalIODir,
+		ExternalIODirConfig: ts.params.ExternalIODirConfig,
+		ForceInsecure:       ts.Insecure(),
+		UseDatabase:         ts.params.UseDatabase,
+		SSLCertsDir:         ts.params.SSLCertsDir,
+		TestingKnobs:        ts.params.Knobs,
 	}
 
 	// Since we're creating a tenant, it doesn't make sense to pass through the
@@ -895,16 +894,6 @@ func (ts *TestServer) StartTenant(
 		baseCfg.HTTPAddr = newAddr
 		baseCfg.HTTPAdvertiseAddr = newAddr
 	}
-	if params.AllowSettingClusterSettings {
-		tenantKnobs, ok := baseCfg.TestingKnobs.TenantTestingKnobs.(*sql.TenantTestingKnobs)
-		if !ok {
-			tenantKnobs = &sql.TenantTestingKnobs{}
-			baseCfg.TestingKnobs.TenantTestingKnobs = tenantKnobs
-		}
-		if tenantKnobs.ClusterSettingsUpdater == nil {
-			tenantKnobs.ClusterSettingsUpdater = st.MakeUpdater()
-		}
-	}
 	sw, err := NewTenantServer(
 		ctx,
 		stopper,

pkg/sql/exec_util.go

@@ -1522,10 +1522,6 @@ func (*PGWireTestingKnobs) ModuleTestingKnobs() {}
 
 // TenantTestingKnobs contains knobs for tenant behavior.
 type TenantTestingKnobs struct {
-	// ClusterSettingsUpdater is a field that if set, allows the tenant to set
-	// in-memory cluster settings. SQL tenants are otherwise prohibited from
-	// setting cluster settings.
-	ClusterSettingsUpdater settings.Updater
 
 	// TenantIDCodecOverride overrides the tenant ID used to construct the SQL
 	// server's codec, but nothing else (e.g. its certs).

pkg/sql/logictest/logic.go

@@ -1377,8 +1377,7 @@ func (t *logicTest) newCluster(
 		t.tenantAddrs = make([]string, cfg.NumNodes)
 		for i := 0; i < cfg.NumNodes; i++ {
 			tenantArgs := base.TestTenantArgs{
-				TenantID:                    serverutils.TestTenantID(),
-				AllowSettingClusterSettings: true,
+				TenantID: serverutils.TestTenantID(),
 				TestingKnobs: base.TestingKnobs{
 					SQLExecutor: &sql.ExecutorTestingKnobs{
 						DeterministicExplain:            true,

pkg/sql/set_cluster_setting.go

@@ -434,15 +434,6 @@ func writeNonDefaultSettingValue(
 		}
 	}
 
-	if knobs := execCfg.TenantTestingKnobs; knobs != nil && knobs.ClusterSettingsUpdater != nil {
-		encVal := settings.EncodedValue{
-			Value: encoded,
-			Type:  setting.Typ(),
-		}
-		if err := execCfg.TenantTestingKnobs.ClusterSettingsUpdater.Set(ctx, name, encVal); err != nil {
-			return reportedValue, expectedEncodedValue, err
-		}
-	}
 	return reportedValue, expectedEncodedValue, nil
 }
 

pkg/sql/sqltestutils/telemetry.go

@@ -175,9 +175,8 @@ func (tt *telemetryTest) Start(t *testing.T, serverArgs []base.TestServerArgs) {
 	tt.prepareCluster(tt.serverDB)
 
 	tt.tenant, tt.tenantDB = serverutils.StartTenant(tt.t, tt.server, base.TestTenantArgs{
-		TenantID:                    serverutils.TestTenantID(),
-		AllowSettingClusterSettings: true,
-		TestingKnobs:                mapServerArgs[0].Knobs,
+		TenantID:     serverutils.TestTenantID(),
+		TestingKnobs: mapServerArgs[0].Knobs,
 	})
 	tt.prepareCluster(tt.tenantDB)
 }