From 412e480b44db36b0e04fca15708a30f5c0e76327 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Fri, 2 May 2025 00:02:50 -0400 Subject: [PATCH] engine: local: Get the logic right 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. --- engine/local/local.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/engine/local/local.go b/engine/local/local.go index 620898d8..9040aea7 100644 --- a/engine/local/local.go +++ b/engine/local/local.go @@ -144,7 +144,7 @@ func (obj *Value) ValueGet(ctx context.Context, key string) (interface{}, error) var val interface{} //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 if err != nil { // 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() 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 // written or it doesn't matter because we're cancelling, meaning we're // shutting down, so our local cache can be invalidated anyways.