Skip to content

Commit

Permalink
settings: move the 'modified' map to valueContainer
Browse files Browse the repository at this point in the history
In a later commit, we will want to ensure that `.Override()` sets the
modified bit. This is not possible if the modified bits are in the
Updater. (The `Override` method is on the setting itself and has no
access to the updater.)

Release note: None
  • Loading branch information
knz committed Sep 25, 2023
1 parent cb7c66f commit 2f5d717
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 18 deletions.
14 changes: 5 additions & 9 deletions pkg/settings/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Expand Down
16 changes: 7 additions & 9 deletions pkg/settings/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -92,7 +91,6 @@ func NewUpdater(sv *Values) Updater {
return NoopUpdater{}
}
return updater{
m: make(map[InternalKey]struct{}, len(registry)),
sv: sv,
}
}
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/settings/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}) {
Expand Down

0 comments on commit 2f5d717

Please sign in to comment.