From f13f00829ce6975c3ee5af74ceb867e3fc4432d8 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 6 Jun 2023 22:25:02 +0000 Subject: [PATCH] settings: track the origin of the current values 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. Release note: none. Epic: none. --- pkg/server/settingswatcher/settings_watcher.go | 13 ++++++++----- pkg/settings/common.go | 11 +++++++++++ pkg/settings/setting.go | 17 +++++++++++++++++ pkg/settings/updater.go | 18 ++++++++++++++++++ pkg/settings/values.go | 10 ++++++++++ 5 files changed, 64 insertions(+), 5 deletions(-) diff --git a/pkg/server/settingswatcher/settings_watcher.go b/pkg/server/settingswatcher/settings_watcher.go index 710de8ed03e1..0154c5bd864b 100644 --- a/pkg/server/settingswatcher/settings_watcher.go +++ b/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) } diff --git a/pkg/settings/common.go b/pkg/settings/common.go index 24da5a7dead5..a7987626f540 100644 --- a/pkg/settings/common.go +++ b/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. diff --git a/pkg/settings/setting.go b/pkg/settings/setting.go index e7ba6112c1d2..72455441c783 100644 --- a/pkg/settings/setting.go +++ b/pkg/settings/setting.go @@ -79,6 +79,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 +145,17 @@ 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 +) diff --git a/pkg/settings/updater.go b/pkg/settings/updater.go index 4796e01728b7..daf2db006dde 100644 --- a/pkg/settings/updater.go +++ b/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) + } +} diff --git a/pkg/settings/values.go b/pkg/settings/values.go index 3a953a7a4642..4ad26ef58ea9 100644 --- a/pkg/settings/values.go +++ b/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{}) {