Skip to content

Expand singleflight boundary in ValidatingCache.Get to cover the full hit+check and miss+load path #4729

@yrobla

Description

@yrobla

Context

Raised as a suggestion on the ValidatingCache.Get implementation.

Currently, singleflight is used only to deduplicate concurrent cache misses (the load path). The cache hit + liveness check path runs outside the singleflight group, meaning concurrent goroutines can independently trigger storage.Load for liveness checks on the same key.

Problem

The current design requires ContainsOrAdd + a follow-up Get to handle the race between concurrent loaders, adding complexity:

ok, _ := c.lruCache.ContainsOrAdd(key, v)
if ok {
    winner, found := c.lruCache.Get(key)
    if !found {
        // Winner was evicted between ContainsOrAdd and Get; keep our freshly loaded value
        return result{v: v}, nil
    }
    if c.onEvict != nil {
        c.onEvict(key, v)
    }
    return result{v: winner}, nil
}

This race-handling logic exists solely because multiple goroutines can reach the miss+load path concurrently.

Suggestion

Move the singleflight boundary to wrap the entire Get — both the hit+check and miss+load paths. With only one goroutine running per key at a time:

  • The miss path can use a plain Add instead of ContainsOrAdd — the concurrent-writer race disappears entirely
  • The ContainsOrAdd + Get race-handling block can be removed
  • Concurrent liveness checks are coalesced into a single storage.Load round-trip per key, reducing storage pressure under concurrent access

Acceptance Criteria

  • singleflight wraps the full Get operation (hit+check and miss+load)
  • ContainsOrAdd is replaced with a plain Add in the miss path
  • The concurrent-writer race-handling block (winner, found := c.lruCache.Get(key)) is removed
  • Concurrent liveness checks on the same key produce a single storage.Load call
  • Existing tests continue to pass

References

  • PR where the current ContainsOrAdd pattern was introduced

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestgoPull requests that update go codescalabilityItems related to scalabilityvmcpVirtual MCP Server related issues

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions