Merge #38309 #76909

38309: docs: add an Insights into Constraint Conformance RFC r=andreimatei a=andreimatei

The set of features described here aim to provide admins with visibility into
aspects of replication and replica placement. In particular admins will be able
to access a report that details which replication constraints are violated.

Release justification: non-code change
Release note: None

76909: settings: disallow tenant from changing overridden settings r=ajwerner a=RaduBerinde

Informs #73857.

#### settings: simplify default overrides

This commit cleans up the Override() implementation, using a
single `map[slotIdx]interface{}`.

Release note: None

#### settings: disallow tenant from changing overridden settings

We now produce an error if a tenant tries to change a cluster setting
that is currently overridden by the operator. The previous behavior
was too subtle: we would change the tenant's "opinion" on the setting,
which would only take effect if/when the operator override was
removed.

To achieve this we query the settings watcher which always has the
current list of overrides. In the future, we should somehow integrate
this information in `settings.Values`.

Release justification: bug fixes and low-risk updates to new functionality

Release note: None

Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
Co-authored-by: Radu Berinde <radu@cockroachlabs.com>

pkg/ccl/logictestccl/testdata/logic_test/tenant_settings

@@ -57,6 +57,13 @@ SHOW CLUSTER SETTING sql.notices.enabled
 ----
 true
 
+# Verify that we disallow setting a TenantWritable setting that is overridden.
+statement error cluster setting 'sql.notices.enabled' is currently overridden by the operator
+SET CLUSTER SETTING sql.notices.enabled = false
+
+statement error cluster setting 'sql.notices.enabled' is currently overridden by the operator
+RESET CLUSTER SETTING sql.notices.enabled
+
 user host-cluster-root
 
 # Remove the all-tenant override; should make no difference.
@@ -118,4 +125,4 @@ SELECT crdb_internal.destroy_tenant(1234, true)
 query I
 SELECT count(*) FROM system.tenant_settings WHERE tenant_id = 1234
 ----
-0
+0

pkg/server/settingswatcher/settings_watcher.go

@@ -95,6 +95,7 @@ func NewWithOverrides(
 ) *SettingsWatcher {
 	s := New(clock, codec, settingsToUpdate, f, stopper, storage)
 	s.overridesMonitor = overridesMonitor
+	settingsToUpdate.OverridesInformer = s
 	return s
 }
 
@@ -360,3 +361,11 @@ func (s *SettingsWatcher) resetUpdater() {
 func (s *SettingsWatcher) SetTestingKnobs(knobs *rangefeedcache.TestingKnobs) {
 	s.testingWatcherKnobs = knobs
 }
+
+// IsOverridden implements cluster.OverridesInformer.
+func (s *SettingsWatcher) IsOverridden(settingName string) bool {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	_, exists := s.mu.overrides[settingName]
+	return exists
+}

pkg/settings/bool.go

@@ -60,12 +60,7 @@ var _ = (*BoolSetting).Default
 // For testing usage only.
 func (b *BoolSetting) Override(ctx context.Context, sv *Values, v bool) {
 	b.set(ctx, sv, v)
-
-	vInt := int64(0)
-	if v {
-		vInt = 1
-	}
-	sv.setDefaultOverrideInt64(b.slot, vInt)
+	sv.setDefaultOverride(b.slot, v)
 }
 
 func (b *BoolSetting) set(ctx context.Context, sv *Values, v bool) {
@@ -78,9 +73,8 @@ func (b *BoolSetting) set(ctx context.Context, sv *Values, v bool) {
 
 func (b *BoolSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
-	ok, val, _ := sv.getDefaultOverride(b.slot)
-	if ok {
-		b.set(ctx, sv, val > 0)
+	if val := sv.getDefaultOverride(b.slot); val != nil {
+		b.set(ctx, sv, val.(bool))
 		return
 	}
 	b.set(ctx, sv, b.defaultValue)

pkg/settings/cluster/cluster_settings.go

@@ -54,6 +54,19 @@ type Settings struct {
 	// Cache can be used for arbitrary caching, e.g. to cache decoded
 	// enterprises licenses for utilccl.CheckEnterpriseEnabled().
 	Cache sync.Map
+
+	// OverridesInformer can be nil.
+	OverridesInformer OverridesInformer
+}
+
+// OverridesInformer is an interface that can be used to figure out if a setting
+// is currently being overridden by the host cluster (only possible for
+// secondary tenants).
+//
+// TODO(radu): move this functionality into settings.Values, provide a way to
+// obtain it along with the current value consistently.
+type OverridesInformer interface {
+	IsOverridden(settingName string) bool
 }
 
 // TelemetryOptOut is a place for controlling whether to opt out of telemetry or not.

pkg/settings/duration.go

@@ -89,7 +89,7 @@ func (d *DurationSetting) Validate(v time.Duration) error {
 // For testing usage only.
 func (d *DurationSetting) Override(ctx context.Context, sv *Values, v time.Duration) {
 	sv.setInt64(ctx, d.slot, int64(v))
-	sv.setDefaultOverrideInt64(d.slot, int64(v))
+	sv.setDefaultOverride(d.slot, v)
 }
 
 func (d *DurationSetting) set(ctx context.Context, sv *Values, v time.Duration) error {
@@ -102,11 +102,10 @@ func (d *DurationSetting) set(ctx context.Context, sv *Values, v time.Duration)
 
 func (d *DurationSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
-	ok, val, _ := sv.getDefaultOverride(d.slot)
-	if ok {
+	if val := sv.getDefaultOverride(d.slot); val != nil {
 		// As per the semantics of override, these values don't go through
 		// validation.
-		_ = d.set(ctx, sv, time.Duration(val))
+		_ = d.set(ctx, sv, val.(time.Duration))
 		return
 	}
 	if err := d.set(ctx, sv, d.defaultValue); err != nil {

pkg/settings/float.go

@@ -68,7 +68,7 @@ func (f *FloatSetting) Override(ctx context.Context, sv *Values, v float64) {
 	if err := f.set(ctx, sv, v); err != nil {
 		panic(err)
 	}
-	sv.setDefaultOverrideInt64(f.slot, int64(math.Float64bits(v)))
+	sv.setDefaultOverride(f.slot, v)
 }
 
 // Validate that a value conforms with the validation function.
@@ -91,11 +91,10 @@ func (f *FloatSetting) set(ctx context.Context, sv *Values, v float64) error {
 
 func (f *FloatSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
-	ok, val, _ := sv.getDefaultOverride(f.slot)
-	if ok {
+	if val := sv.getDefaultOverride(f.slot); val != nil {
 		// As per the semantics of override, these values don't go through
 		// validation.
-		_ = f.set(ctx, sv, math.Float64frombits(uint64((val))))
+		_ = f.set(ctx, sv, val.(float64))
 		return
 	}
 	if err := f.set(ctx, sv, f.defaultValue); err != nil {

pkg/settings/int.go

@@ -75,7 +75,7 @@ func (i *IntSetting) Validate(v int64) error {
 // For testing usage only.
 func (i *IntSetting) Override(ctx context.Context, sv *Values, v int64) {
 	sv.setInt64(ctx, i.slot, v)
-	sv.setDefaultOverrideInt64(i.slot, v)
+	sv.setDefaultOverride(i.slot, v)
 }
 
 func (i *IntSetting) set(ctx context.Context, sv *Values, v int64) error {
@@ -88,11 +88,10 @@ func (i *IntSetting) set(ctx context.Context, sv *Values, v int64) error {
 
 func (i *IntSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
-	ok, val, _ := sv.getDefaultOverride(i.slot)
-	if ok {
+	if val := sv.getDefaultOverride(i.slot); val != nil {
 		// As per the semantics of override, these values don't go through
 		// validation.
-		_ = i.set(ctx, sv, val)
+		_ = i.set(ctx, sv, val.(int64))
 		return
 	}
 	if err := i.set(ctx, sv, i.defaultValue); err != nil {

pkg/settings/values.go

@@ -32,13 +32,12 @@ type Values struct {
 
 	nonSystemTenant bool
 
-	overridesMu struct {
+	defaultOverridesMu struct {
 		syncutil.Mutex
+
 		// defaultOverrides maintains the set of overridden default values (see
 		// Override()).
-		defaultOverrides valuesContainer
-		// setOverrides is the list of slots with values in defaultOverrides.
-		setOverrides map[slotIdx]struct{}
+		defaultOverrides map[slotIdx]interface{}
 	}
 
 	changeMu struct {
@@ -155,37 +154,23 @@ func (sv *Values) setInt64(ctx context.Context, slot slotIdx, newVal int64) {
 	}
 }
 
-// setDefaultOverrideInt64 overrides the default value for the respective
-// setting to newVal.
-func (sv *Values) setDefaultOverrideInt64(slot slotIdx, newVal int64) {
-	sv.overridesMu.Lock()
-	defer sv.overridesMu.Unlock()
-	sv.overridesMu.defaultOverrides.setInt64Val(slot, newVal)
-	sv.setDefaultOverrideLocked(slot)
+// setDefaultOverride overrides the default value for the respective setting to
+// newVal.
+func (sv *Values) setDefaultOverride(slot slotIdx, newVal interface{}) {
+	sv.defaultOverridesMu.Lock()
+	defer sv.defaultOverridesMu.Unlock()
+	if sv.defaultOverridesMu.defaultOverrides == nil {
+		sv.defaultOverridesMu.defaultOverrides = make(map[slotIdx]interface{})
+	}
+	sv.defaultOverridesMu.defaultOverrides[slot] = newVal
 }
 
-// setDefaultOverrideLocked marks the slot as having an overridden default value.
-func (sv *Values) setDefaultOverrideLocked(slot slotIdx) {
-	if sv.overridesMu.setOverrides == nil {
-		sv.overridesMu.setOverrides = make(map[slotIdx]struct{})
-	}
-	sv.overridesMu.setOverrides[slot] = struct{}{}
-}
-
-// getDefaultOverrides checks whether there's a default override for slotIdx-1.
-// If there isn't, the first ret val is false. Otherwise, the first ret val is
-// true, the second is the int64 override and the last is a pointer to the
-// generic value override. Callers are expected to only use the override value
-// corresponding to their setting type.
-func (sv *Values) getDefaultOverride(slot slotIdx) (bool, int64, *atomic.Value) {
-	sv.overridesMu.Lock()
-	defer sv.overridesMu.Unlock()
-	if _, ok := sv.overridesMu.setOverrides[slot]; !ok {
-		return false, 0, nil
-	}
-	return true,
-		sv.overridesMu.defaultOverrides.intVals[slot],
-		&sv.overridesMu.defaultOverrides.genericVals[slot]
+// getDefaultOverrides checks whether there's a default override for slotIdx-1
+// and returns it (or nil if there is no override).
+func (sv *Values) getDefaultOverride(slot slotIdx) interface{} {
+	sv.defaultOverridesMu.Lock()
+	defer sv.defaultOverridesMu.Unlock()
+	return sv.defaultOverridesMu.defaultOverrides[slot]
 }
 
 func (sv *Values) setGeneric(ctx context.Context, slot slotIdx, newVal interface{}) {

pkg/sql/set_cluster_setting.go

@@ -117,6 +117,10 @@ func (p *planner) SetClusterSetting(
 		}
 	}
 
+	if st.OverridesInformer != nil && st.OverridesInformer.IsOverridden(name) {
+		return nil, errors.Errorf("cluster setting '%s' is currently overridden by the operator", name)
+	}
+
 	var value tree.TypedExpr
 	if n.Value != nil {
 		// For DEFAULT, let the value reference be nil. That's a RESET in disguise.