Skip to content

Commit 2f823af

Browse files
committed
upgrades: swallow setting override error in diagnostics migration
Previously if a tenant ran the optInToDiagnosticsStatReporting migration, which sets diagnostics.reporting.enabled, after the system tenant already overrode this setting, the migration would enter a fail loop. With this patch, the migration instead noops. Epic: none Release note: none
1 parent 45057e1 commit 2f823af

File tree

5 files changed

+14
-4
lines changed

5 files changed

+14
-4
lines changed

pkg/ccl/logictestccl/testdata/logic_test/tenant_settings

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,10 @@ SHOW CLUSTER SETTING sql.notices.enabled
109109
true
110110

111111
# Verify that we disallow setting a ApplicationLevel setting that is overridden.
112-
statement error cluster setting 'sql.notices.enabled' is currently overridden by the operator
112+
statement error cluster setting 'sql.notices.enabled' cannot be set: cluster setting is overridden by system virtual cluster
113113
SET CLUSTER SETTING sql.notices.enabled = false
114114

115-
statement error cluster setting 'sql.notices.enabled' is currently overridden by the operator
115+
statement error cluster setting 'sql.notices.enabled' cannot be set: cluster setting is overridden by system virtual cluster
116116
RESET CLUSTER SETTING sql.notices.enabled
117117

118118
user host-cluster-root

pkg/crosscluster/physical/standby_read_ts_poller_job_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,19 @@ func TestStandbyReadTSPollerJob(t *testing.T) {
4343
jobutils.WaitForJobToRun(c.T, c.DestSysSQL, jobspb.JobID(ingestionJobID))
4444
t.Logf("test setup took %s", timeutil.Since(beginTS))
4545

46+
readerTenantName := fmt.Sprintf("%s-readonly", args.DestTenantName)
47+
48+
// Ensures the reader tenant can spin up even if the system tenant overrode
49+
// the diagnostics setting, set during a permanent migration.
50+
c.DestSysSQL.Exec(t, fmt.Sprintf("ALTER TENANT '%s' SET CLUSTER SETTING diagnostics.reporting.enabled = true", readerTenantName))
51+
4652
srcTime := c.SrcCluster.Server(0).Clock().Now()
4753
c.WaitUntilReplicatedTime(srcTime, jobspb.JobID(ingestionJobID))
4854

4955
stats := replicationtestutils.TestingGetStreamIngestionStatsFromReplicationJob(t, ctx, c.DestSysSQL, ingestionJobID)
5056
readerTenantID := stats.IngestionDetails.ReadTenantID
5157
require.NotNil(t, readerTenantID)
5258

53-
readerTenantName := fmt.Sprintf("%s-readonly", args.DestTenantName)
5459
c.ConnectToReaderTenant(ctx, readerTenantID, readerTenantName)
5560

5661
defaultDBQuery := `

pkg/settings/cluster/cluster_settings.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ type OverridesInformer interface {
6262
IsOverridden(settingKey settings.InternalKey) bool
6363
}
6464

65+
var SettingOverrideErr = errors.New("cluster setting is overridden by system virtual cluster")
66+
6567
// TelemetryOptOut controls whether to opt out of telemetry (including Sentry) or not.
6668
var TelemetryOptOut = envutil.EnvOrDefaultBool("COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING", false)
6769

pkg/sql/set_cluster_setting.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ func (p *planner) SetClusterSetting(
225225
}
226226

227227
if st.OverridesInformer != nil && st.OverridesInformer.IsOverridden(setting.InternalKey()) {
228-
return nil, errors.Errorf("cluster setting '%s' is currently overridden by the operator", name)
228+
return nil, errors.Wrapf(cluster.SettingOverrideErr, "cluster setting '%s' cannot be set", name)
229229
}
230230

231231
value, err := p.getAndValidateTypedClusterSetting(ctx, name, n.Value, setting)

pkg/upgrade/upgrades/permanent_upgrades.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ func optInToDiagnosticsStatReporting(
168168
_, err := deps.InternalExecutor.Exec(
169169
ctx, "optInToDiagnosticsStatReporting", nil, /* txn */
170170
`SET CLUSTER SETTING diagnostics.reporting.enabled = true`)
171+
if errors.Is(err, cluster.SettingOverrideErr) {
172+
return nil
173+
}
171174
return err
172175
}
173176

0 commit comments

Comments
 (0)