Merge cockroachdb#104449

104449: sql: add default_value and origin to show cluster settings r=maryliag a=maryliag

Fixes https://cockroachlabs.atlassian.net/browse/CRDB-28519

**Commit 1**
This adds an API to the settings package to track the origin of the current value of each setting, such as whether it is set by the the default value, an explicit override or an external override.

**Commit 2**
Previously, there was no easy way to see default values for
cluster settings. This commit add the column for `default_value`
and `origin` to `crdb_internal.cluster_settings` and
the `show cluster settings` command.

Release note (sql change): Add columns `default_value` and
`origin` (default, override, external-override) to the
`show cluster settings` command.

Co-authored-by: David Taylor <[email protected]>
Co-authored-by: maryliag <[email protected]>

Author: 3 people

pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant

@@ -224,10 +224,10 @@ SELECT * FROM crdb_internal.session_trace WHERE span_idx < 0
 ----
 span_idx  message_idx  timestamp  duration  operation  loc  tag  message age
 
-query TTTBT colnames
+query TTTBTTT colnames
 SELECT * FROM crdb_internal.cluster_settings WHERE variable = ''
 ----
-variable  value  type  public  description
+variable  value  type  public  description default_value origin
 
 query TI colnames
 SELECT * FROM crdb_internal.feature_usage WHERE feature_name = ''

pkg/server/settings_test.go

@@ -276,7 +276,7 @@ func TestSettingsShowAll(t *testing.T) {
 	if len(rows) < 2 {
 		t.Fatalf("show all returned too few rows (%d)", len(rows))
 	}
-	const expColumns = 5
+	const expColumns = 7
 	if len(rows[0]) != expColumns {
 		t.Fatalf("show all must return %d columns, found %d", expColumns, len(rows[0]))
 	}

pkg/server/settingswatcher/settings_watcher.go

@@ -292,7 +292,7 @@ func (s *SettingsWatcher) maybeSet(ctx context.Context, name string, sv settings
 		}
 	} else {
 		if !hasOverride {
-			s.setLocked(ctx, name, sv.val)
+			s.setLocked(ctx, name, sv.val, settings.OriginExplicitlySet)
 		}
 	}
 }
@@ -309,7 +309,9 @@ type settingsValue struct {
 const versionSettingKey = "version"
 
 // set the current value of a setting.
-func (s *SettingsWatcher) setLocked(ctx context.Context, key string, val settings.EncodedValue) {
+func (s *SettingsWatcher) setLocked(
+	ctx context.Context, key string, val settings.EncodedValue, origin settings.ValueOrigin,
+) {
 	// Both the system tenant and secondary tenants no longer use this code
 	// path to propagate cluster version changes (they rely on
 	// BumpClusterVersion instead). The secondary tenants however, still rely
@@ -335,6 +337,7 @@ func (s *SettingsWatcher) setLocked(ctx context.Context, key string, val setting
 	if err := s.mu.updater.Set(ctx, key, val); err != nil {
 		log.Warningf(ctx, "failed to set setting %s to %s: %v", redact.Safe(key), val.Value, err)
 	}
+	s.mu.updater.SetValueOrigin(ctx, key, origin)
 }
 
 // setDefaultLocked sets a setting to its default value.
@@ -348,7 +351,7 @@ func (s *SettingsWatcher) setDefaultLocked(ctx context.Context, key string) {
 		Value: setting.EncodedDefault(),
 		Type:  setting.Typ(),
 	}
-	s.setLocked(ctx, key, val)
+	s.setLocked(ctx, key, val, settings.OriginDefault)
 }
 
 // updateOverrides updates the overrides map and updates any settings
@@ -384,7 +387,7 @@ func (s *SettingsWatcher) updateOverrides(ctx context.Context) {
 		}
 		// A new override was added or an existing override has changed.
 		s.mu.overrides[key] = val
-		s.setLocked(ctx, key, val)
+		s.setLocked(ctx, key, val, settings.OriginExternallySet)
 	}
 
 	// Clean up any overrides that were removed.
@@ -395,7 +398,7 @@ func (s *SettingsWatcher) updateOverrides(ctx context.Context) {
 			// Reset the setting to the value in the settings table (or the default
 			// value).
 			if sv, ok := s.mu.values[key]; ok && !sv.tombstone {
-				s.setLocked(ctx, key, sv.val)
+				s.setLocked(ctx, key, sv.val, settings.OriginExplicitlySet)
 			} else {
 				s.setDefaultLocked(ctx, key)
 			}

pkg/settings/common.go

@@ -74,6 +74,15 @@ func (c *common) ErrorHint() (bool, string) {
 	return false, ""
 }
 
+func (c *common) getSlot() slotIdx {
+	return c.slot
+}
+
+// ValueOrigin returns the origin of the current value of the setting.
+func (c *common) ValueOrigin(ctx context.Context, sv *Values) ValueOrigin {
+	return sv.getValueOrigin(ctx, c.slot)
+}
+
 // SetReportable indicates whether a setting's value can show up in SHOW ALL
 // CLUSTER SETTINGS and telemetry reports.
 //
@@ -114,6 +123,8 @@ type internalSetting interface {
 	isRetired() bool
 	setToDefault(ctx context.Context, sv *Values)
 
+	getSlot() slotIdx
+
 	// isReportable indicates whether the value of the setting can be
 	// included in user-facing reports such as that produced by SHOW ALL
 	// CLUSTER SETTINGS.

pkg/settings/setting.go

@@ -10,7 +10,10 @@
 
 package settings
 
-import "context"
+import (
+	"context"
+	"fmt"
+)
 
 // Setting is the interface exposing the metadata for a cluster setting.
 //
@@ -79,6 +82,9 @@ type NonMaskedSetting interface {
 	// ErrorHint returns a hint message to be displayed to the user when there's
 	// an error.
 	ErrorHint() (bool, string)
+
+	// ValueOrigin returns the origin of the current value.
+	ValueOrigin(ctx context.Context, sv *Values) ValueOrigin
 }
 
 // Class describes the scope of a setting in multi-tenant scenarios. While all
@@ -142,3 +148,24 @@ const (
 	// In short: "Go ahead but be careful."
 	Public
 )
+
+// ValueOrigin indicates the origin of the current value of a setting, e.g. if
+// it is coming from the in-code default or an explicit override.
+type ValueOrigin uint32
+
+const (
+	// OriginDefault indicates the value in use is the default value.
+	OriginDefault ValueOrigin = iota
+	// OriginExplicitlySet indicates the value is has been set explicitly.
+	OriginExplicitlySet
+	// OriginExternallySet indicates the value has been set externally, such as
+	// via a host-cluster override for this or all tenant(s).
+	OriginExternallySet
+)
+
+func (v ValueOrigin) String() string {
+	if v > OriginExternallySet {
+		return fmt.Sprintf("invalid (%d)", v)
+	}
+	return [...]string{"default", "override", "external-override"}[v]
+}

pkg/settings/updater.go

@@ -72,6 +72,7 @@ type updater struct {
 type Updater interface {
 	Set(ctx context.Context, key string, value EncodedValue) error
 	ResetRemaining(ctx context.Context)
+	SetValueOrigin(ctx context.Context, key string, origin ValueOrigin)
 }
 
 // A NoopUpdater ignores all updates.
@@ -83,6 +84,8 @@ func (u NoopUpdater) Set(ctx context.Context, key string, value EncodedValue) er
 // ResetRemaining implements Updater. It is a no-op.
 func (u NoopUpdater) ResetRemaining(context.Context) {}
 
+func (u NoopUpdater) SetValueOrigin(ctx context.Context, key string, origin ValueOrigin) {}
+
 // NewUpdater makes an Updater.
 func NewUpdater(sv *Values) Updater {
 	if ignoreAllUpdates {
@@ -165,6 +168,13 @@ func (u updater) Set(ctx context.Context, key string, value EncodedValue) error
 // ResetRemaining sets all settings not updated by the updater to their default values.
 func (u updater) ResetRemaining(ctx context.Context) {
 	for k, v := range registry {
+
+		if _, hasOverride := u.m[k]; hasOverride {
+			u.sv.setValueOrigin(ctx, v.getSlot(), OriginExplicitlySet)
+		} else {
+			u.sv.setValueOrigin(ctx, v.getSlot(), OriginDefault)
+		}
+
 		if u.sv.NonSystemTenant() && v.Class() == SystemOnly {
 			// Don't try to reset system settings on a non-system tenant.
 			continue
@@ -174,3 +184,11 @@ func (u updater) ResetRemaining(ctx context.Context) {
 		}
 	}
 }
+
+// SetValueOrigin sets the origin of the value of a given setting.
+func (u updater) SetValueOrigin(ctx context.Context, key string, origin ValueOrigin) {
+	d, ok := registry[key]
+	if ok {
+		u.sv.setValueOrigin(ctx, d.getSlot(), origin)
+	}
+}

pkg/settings/values.go

@@ -61,6 +61,8 @@ type valuesContainer struct {
 	// current context (i.e. it is a SystemOnly setting and the container is for a
 	// tenant). Reading or writing such a setting causes panics in test builds.
 	forbidden [numSlots]bool
+
+	hasValue [numSlots]uint32
 }
 
 func (c *valuesContainer) setGenericVal(slot slotIdx, newVal interface{}) {
@@ -154,6 +156,14 @@ func (sv *Values) setInt64(ctx context.Context, slot slotIdx, newVal int64) {
 	}
 }
 
+func (sv *Values) setValueOrigin(ctx context.Context, slot slotIdx, origin ValueOrigin) {
+	atomic.StoreUint32(&sv.container.hasValue[slot], uint32(origin))
+}
+
+func (sv *Values) getValueOrigin(ctx context.Context, slot slotIdx) ValueOrigin {
+	return ValueOrigin(atomic.LoadUint32(&sv.container.hasValue[slot]))
+}
+
 // setDefaultOverride overrides the default value for the respective setting to
 // newVal.
 func (sv *Values) setDefaultOverride(slot slotIdx, newVal interface{}) {

pkg/sql/crdb_internal.go

@@ -2054,7 +2054,9 @@ CREATE TABLE crdb_internal.cluster_settings (
   value         STRING NOT NULL,
   type          STRING NOT NULL,
   public        BOOL NOT NULL, -- whether the setting is documented, which implies the user can expect support.
-  description   STRING NOT NULL
+  description   STRING NOT NULL,
+  default_value STRING NOT NULL,
+  origin        STRING NOT NULL -- the origin of the value: 'default' , 'override' or 'external-override'
 )`,
 	populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error {
 		hasSqlModify, err := p.HasPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.MODIFYSQLCLUSTERSETTING, p.User())
@@ -2093,12 +2095,19 @@ CREATE TABLE crdb_internal.cluster_settings (
 			strVal := setting.String(&p.ExecCfg().Settings.SV)
 			isPublic := setting.Visibility() == settings.Public
 			desc := setting.Description()
+			defaultVal, err := setting.DecodeToString(setting.EncodedDefault())
+			if err != nil {
+				return err
+			}
+			origin := setting.ValueOrigin(ctx, &p.ExecCfg().Settings.SV).String()
 			if err := addRow(
 				tree.NewDString(k),
 				tree.NewDString(strVal),
 				tree.NewDString(setting.Typ()),
 				tree.MakeDBool(tree.DBool(isPublic)),
 				tree.NewDString(desc),
+				tree.NewDString(defaultVal),
+				tree.NewDString(origin),
 			); err != nil {
 				return err
 			}

pkg/sql/delegate/show_all_cluster_settings.go

@@ -77,12 +77,12 @@ func (d *delegator) delegateShowClusterSettingList(
 
 	if stmt.All {
 		return d.parse(
-			`SELECT variable, value, type AS setting_type, public, description
+			`SELECT variable, value, type AS setting_type, public, description, default_value, origin
        FROM   crdb_internal.cluster_settings`,
 		)
 	}
 	return d.parse(
-		`SELECT variable, value, type AS setting_type, description
+		`SELECT variable, value, type AS setting_type, description, default_value, origin
      FROM   crdb_internal.cluster_settings
      WHERE  public IS TRUE`,
 	)

pkg/sql/logictest/testdata/logic_test/cluster_settings

@@ -78,6 +78,44 @@ WHERE variable IN ('sql.defaults.default_int_size')
 ----
 sql.defaults.default_int_size  4
 
+query TTTT
+SELECT variable, value, default_value, origin FROM [SHOW ALL CLUSTER SETTINGS]
+WHERE variable IN ('sql.index_recommendation.drop_unused_duration')
+----
+sql.index_recommendation.drop_unused_duration  168h0m0s 168h0m0s default
+
+statement ok
+SET CLUSTER SETTING sql.index_recommendation.drop_unused_duration = '10s'
+
+query TTTT
+SELECT variable, value, default_value, origin FROM [SHOW ALL CLUSTER SETTINGS]
+WHERE variable IN ('sql.index_recommendation.drop_unused_duration')
+----
+sql.index_recommendation.drop_unused_duration  10s 168h0m0s override
+
+statement ok
+RESET CLUSTER SETTING sql.index_recommendation.drop_unused_duration
+
+query TTTT
+SELECT variable, value, default_value, origin FROM [SHOW ALL CLUSTER SETTINGS]
+WHERE variable IN ('sql.index_recommendation.drop_unused_duration')
+----
+sql.index_recommendation.drop_unused_duration  168h0m0s 168h0m0s default
+
+user host-cluster-root
+
+statement ok
+ALTER TENANT ALL SET CLUSTER SETTING sql.index_recommendation.drop_unused_duration = '50s'
+
+user root
+
+onlyif config 3node-tenant-default-configs
+query TTTT
+SELECT variable, value, default_value, origin FROM [SHOW ALL CLUSTER SETTINGS]
+WHERE variable IN ('sql.index_recommendation.drop_unused_duration')
+----
+sql.index_recommendation.drop_unused_duration  50s 168h0m0s external-override
+
 user root
 
 statement ok