Skip to content

fix(yurthub): prevent in-memory cache drift on storage access conflic… - #2731

Open
WorrierKhushal wants to merge 1 commit into
openyurtio:masterfrom
WorrierKhushal:fix/2712-yurthub-watch-drop-cache-inconsistency
Open

fix(yurthub): prevent in-memory cache drift on storage access conflic…#2731
WorrierKhushal wants to merge 1 commit into
openyurtio:masterfrom
WorrierKhushal:fix/2712-yurthub-watch-drop-cache-inconsistency

Conversation

@WorrierKhushal

@WorrierKhushal WorrierKhushal commented Aug 4, 2026

Copy link
Copy Markdown

What type of PR is this?

/kind bug
Fixes #2712

What this PR does / why we need it:

1. Architectural Bug Summary & Impact

In pkg/yurthub/cachemanager/cache_manager.go, when a watch event arrives while ReplaceComponentList() is holding a key-prefix lock on disk storage, storeObjectWithKey() catches storage.ErrStorageAccessConflict.

Previously, storeObjectWithKey() swallowed ErrStorageAccessConflict and returned nil. Following that call, saveWatchObject() unconditionally invoked updateInMemoryCache() regardless of whether the disk write succeeded.

The Bug: The in-memory cache was updated with the newer resource version from the watch event, but the disk write was silently dropped. This created a persistent state drift between memory and disk storage. If YurtHub restarted or an edge node disconnected, the node would fall back to stale disk data while in-memory operations served the newer version.


2. Detailed Code Changes & Explanation

A. Error Propagation in storeObjectWithKey()

We updated storeObjectWithKey() to propagate storage.ErrStorageAccessConflict back to the caller instead of returning nil.

// pkg/yurthub/cachemanager/cache_manager.go

func (cm *cacheManager) storeObjectWithKey(accessor meta.Object: ... ) error {
    ...
    err := cm.storageWrapper.Update(key, obj, rv)
    if err != nil {
        if errors.Is(err, storage.ErrStorageNotFound) {
            if err := cm.storageWrapper.Create(key, obj); err != nil {
                if errors.Is(err, storage.ErrStorageAccessConflict) {
                    klog.V(2).Infof("storage access conflict for %s, skip create: %v", key, err)
                    // BEFORE: return nil
                    // AFTER:
                    return err
                }
                return err
            }
            return nil
        } else if errors.Is(err, storage.ErrStorageAccessConflict) {
            klog.V(2).Infof("storage access conflict for %s, skip update: %v", key, err)
            // BEFORE: return nil
            // AFTER:
            return err
        }
        return err
    }
    return nil
}

@WorrierKhushal
WorrierKhushal requested a review from a team as a code owner August 4, 2026 17:19
@sonarqubecloud

sonarqubecloud Bot commented Aug 4, 2026

Copy link
Copy Markdown

@WorrierKhushal

Copy link
Copy Markdown
Author

Hi maintainers sir @wawlian @carolove @Fei-Guo ,

I have opened a Pull Request to resolve issue #2712 regarding state divergence between YurtHub's in-memory cache and disk storage.

Summary of Root Cause & Fix:

  1. Root Cause: When saveWatchObject() hit a storage.ErrStorageAccessConflict during a concurrent list replace operation, storeObjectWithKey() swallowed the error and returned nil. This caused updateInMemoryCache() to execute anyway, caching a newer ResourceVersion in memory that was never written to disk.
  2. The Fix:
    • Propagated ErrStorageAccessConflict from storeObjectWithKey().
    • Wrapped updateInMemoryCache() in saveWatchObject() with an if err == nil check (matching saveOneObject()).
    • Adjusted the fallback log level in saveWatchObject() for conflict errors to klog.V(2) to avoid duplicate error log noise.
  3. Testing: Added a fully deterministic unit test (TestCacheWatchResponseKeepsInMemoryCacheConsistentWithDiskOnAccessConflict) using channel synchronization to test concurrent List/Watch events without time.Sleep.

Please take a look when you have a chance. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Silent watch event drop during disk storage list-replacement causes permanent cache inconsistency between memory and disk

1 participant