[ML] Fix concurrent LFU cache count invariant under lock timeouts

[edsavage]

Summary

testConcurrentReadsAndWrites (in CCompressedLfuCacheTest) intermittently fails on loaded debug CI agents with:

Count mismatch 1097 (less 7 lost) vs 1092.
critical check cache.checkInvariants() has failed

Root cause

CCompressedLfuCache::lookup incremented m_NumberLookups inside the read-guard lambda, which does not run when the read lock acquisition times out (m_MaximumWait is only 50 ms). When the read lock times out the lookup falls through to the miss path, which can still:

• insert a value (+1 to the tracked counts), or
• find the key already cached and increment its count (+1), or
• time out on the write lock and bump m_LostCount.

In every case the write side changed the bookkeeping without the matching lookup being counted, so the surviving counts could exceed m_NumberLookups - m_LostCount, tripping the strict-equality check in checkInvariants(). This is a benign accounting asymmetry, not cache corruption, but it produces a real flaky test under contention.

There is also an inherent, opposite race: an item seen as a hit under the read lock can be evicted before the write lock increments its count, so counts can also legitimately be lost. The pre-existing != (strict equality) check was fragile in both directions.

Fix

• Count every lookup exactly once, before taking the read lock, so a read-lock timeout no longer desynchronises the lookup count from the write-side contributions.
• Relax the count invariant to a one-sided bound for the timeout-capable (concurrent) cache: counts may be lost but must never be fabricated (totalCount <= accountedLookups). The exact equality is retained for the single-threaded cache, which has no timeouts or races. The structural invariants (size / link / memory) remain strict.

Test

Adds testConcurrentCountInvariantWithTimeouts, which drives read- and write-lock timeouts via a zero maximum wait under heavy contention. It fails 20/20 against the previous code and passes with the fix.

Test plan

• testConcurrentReadsAndWrites passes
• New testConcurrentCountInvariantWithTimeouts passes with the fix and fails reliably (20/20) without it
• Single-threaded cases (testLookup, testMemoryUsage, testResize) still pass (strict-equality invariant path unchanged)

Made with Cursor

[elasticsearchmachine]

Pinging @elastic/ml-core (Team:ML)

[Copilot]

Pull request overview

This PR fixes a flaky invariant check in core::CCompressedLfuCache under contention by ensuring lookups are always accounted for (even when lock acquisition times out) and by making the invariant tolerant of lost counts in the timeout-capable concurrent cache.

Changes:

• Move m_NumberLookups increment outside the read-guard lambda so read-lock timeouts can’t desynchronise lookup accounting.
• Relax the count invariant for the concurrent (timeout-capable) cache to require a one-sided bound (totalCount <= accountedLookups) while keeping strict equality for the non-timeout cache.
• Add a new stress/regression test that forces lock timeouts (maximumWait = 0ms) and validates invariants remain satisfied.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
lib/core/unittest/CCompressedLfuCacheTest.cc	Adds a regression test that heavily contends the concurrent cache with zero lock wait to ensure the invariant remains stable under timeouts.
include/core/CCompressedLfuCache.h	Fixes lookup counting under lock timeouts and relaxes the invariant appropriately for the concurrent cache.
docs/changelog/3090.yaml	Adds a changelog entry documenting the bug fix.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.