diff --git a/pkg/settings/settings_test.go b/pkg/settings/settings_test.go index e48d1fbd6336..293ef65009a4 100644 --- a/pkg/settings/settings_test.go +++ b/pkg/settings/settings_test.go @@ -574,23 +574,19 @@ func TestCache(t *testing.T) { if expected, actual := true, boolFA.Get(sv); expected != actual { t.Fatalf("expected %v, got %v", expected, actual) } - // If the updater doesn't have a key, e.g. if the setting has been deleted, - // Resetting it from the cache. + // The updated status remains in sv. A new updater is able to pick + // it up. settings.NewUpdater(sv).ResetRemaining(ctx) - if expected, actual := 2, changes.boolTA; expected != actual { + if expected, actual := 1, changes.boolTA; expected != actual { t.Fatalf("expected %d, got %d", expected, actual) } - if expected, actual := 2, changes.i1A; expected != actual { + if expected, actual := 1, changes.i1A; expected != actual { t.Fatalf("expected %d, got %d", expected, actual) } - if expected, actual := false, boolFA.Get(sv); expected != actual { - t.Fatalf("expected %v, got %v", expected, actual) - } - - if expected, actual := false, boolFA.Get(sv); expected != actual { + if expected, actual := true, boolFA.Get(sv); expected != actual { t.Fatalf("expected %v, got %v", expected, actual) } }) diff --git a/pkg/settings/updater.go b/pkg/settings/updater.go index e5023381573a..5efbcf6fbf96 100644 --- a/pkg/settings/updater.go +++ b/pkg/settings/updater.go @@ -60,7 +60,6 @@ func EncodeProtobuf(p protoutil.Message) string { type updater struct { sv *Values - m map[InternalKey]struct{} } // Updater is a helper for updating the in-memory settings. @@ -92,7 +91,6 @@ func NewUpdater(sv *Values) Updater { return NoopUpdater{} } return updater{ - m: make(map[InternalKey]struct{}, len(registry)), sv: sv, } } @@ -108,7 +106,7 @@ func (u updater) Set(ctx context.Context, key InternalKey, value EncodedValue) e return errors.Errorf("unknown setting '%s'", key) } - u.m[key] = struct{}{} + u.sv.container.modified[d.getSlot()] = true if expected := d.Typ(); value.Type != expected { return errors.Errorf("setting '%s' defined as type %s, not %s", d.Name(), expected, value.Type) @@ -167,19 +165,19 @@ func (u updater) Set(ctx context.Context, key InternalKey, value EncodedValue) e // 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) + for _, v := range registry { + slot := v.getSlot() + if hasOverride := u.sv.container.modified[slot]; hasOverride { + u.sv.setValueOrigin(ctx, slot, OriginExplicitlySet) } else { - u.sv.setValueOrigin(ctx, v.getSlot(), OriginDefault) + u.sv.setValueOrigin(ctx, slot, OriginDefault) } if u.sv.SpecializedToVirtualCluster() && v.Class() == SystemOnly { // Don't try to reset system settings on a non-system tenant. continue } - if _, ok := u.m[k]; !ok { + if m := u.sv.container.modified[slot]; !m { v.setToDefault(ctx, u.sv) } } diff --git a/pkg/settings/values.go b/pkg/settings/values.go index 09683e472b11..bfc1a4bd1fa5 100644 --- a/pkg/settings/values.go +++ b/pkg/settings/values.go @@ -87,6 +87,11 @@ type valuesContainer struct { forbidden [numSlots]bool hasValue [numSlots]uint32 + + // modified is set when a setting is explictly set via Set() + // in the updater. + // TODO(knz): Check if this can be merged with hasValue above. + modified [numSlots]bool } func (c *valuesContainer) setGenericVal(slot slotIdx, newVal interface{}) {