lourenco/mgmt
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

engine: local: Get the logic right

Browse Source 
I think we were not benefitting from the cache and sending unnecessary
events. It would be great to have tests for this, but commit this fix
for now, and be embarrassed in the future if I got this code wrong.
This commit is contained in:
James Shubin
2025-05-02 00:02:50 -04:00
parent cc2a235fbb
commit 412e480b44
1 changed files with 11 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
engine/local/local.go
View File

@@ -144,7 +144,7 @@ func (obj *Value) ValueGet(ctx context.Context, key string) (interface{}, error)
var val interface{} var val interface{}
//var err error //var err error
if _, skip := obj.skipread[key]; skip { if _, skip := obj.skipread[key]; !skip {
val, err = valueRead(ctx, prefix, key) // must return val == nil if missing val, err = valueRead(ctx, prefix, key) // must return val == nil if missing
if err != nil { if err != nil {
// We had an actual read issue. Report this and stop // We had an actual read issue. Report this and stop
@@ -177,6 +177,16 @@ func (obj *Value) ValueSet(ctx context.Context, key string, value interface{}) e
obj.mutex.Lock() obj.mutex.Lock()
defer obj.mutex.Unlock() defer obj.mutex.Unlock()
// If we're already in the correct state, then return early and *don't*
// send any events at the very end...
v, exists := obj.values[key]
if !exists && value == nil {
return nil // already in the correct state
}
if exists && v == value { // XXX: reflect.DeepEqual(v, value) ?
return nil // already in the correct state
}
// Write to state dir on disk first. If ctx cancels, we assume it's not // Write to state dir on disk first. If ctx cancels, we assume it's not
// written or it doesn't matter because we're cancelling, meaning we're // written or it doesn't matter because we're cancelling, meaning we're
// shutting down, so our local cache can be invalidated anyways. // shutting down, so our local cache can be invalidated anyways.