SOLR-18506: Fix TestThinCache.testSimple flakiness from evictions in the warming phase - #5029

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

https://issues.apache.org/jira/browse/SOLR-18506

What happens today

TestThinCache.testSimple fails intermittently because warming the second cache can evict one of its own entries from the shared backing cache.

org.apache.solr.search.TestThinCache.testSimple fails intermittently with "expected:<0> but was:<1>" at the second cache's evictions assertion. Recorded 14 times in 30 days across unrelated PRs, ten on branch_9x, and the value was 1 every time. Cause: the second cache warms from the first into a shared backing cache that is already full, so each warming put evicts an entry; the victim usually belongs to the first cache, which this test does not register for removal callbacks, so nothing is counted, but occasionally it is one of the second cache's own entries. ThinCache.warm resets the counter after warming, so only put(103) can leave a count: always exactly 1. Not a timing race: the backing cache uses a same-thread executor, so evictions complete inside the put. The victim depends on key hashes, and the test's scope objects hash by identity, so the result varies per JVM and does not follow the test seed. The commit title says race; the model in Proof shows the eviction is delivered synchronously, so this is not a timing race.

What this change does

The test enlarges the shared backing cache to 200, more than everything it can hold, so no eviction can occur during warming.

Test-only, one file. After the first cache's assertions, the test enlarges the shared backing cache through the existing public CaffeineCache.setMaxSize(200). The test puts 127 keys and holds at most 126 at once, so at capacity 200 nothing can evict during warming and the second cache's eviction count is 0 by construction. The first cache's evictions never enter this count: it is not registered as a removal listener in this test (registration happens in initForSearcher, which the test bypasses by wiring the cache by hand), so the priors copied by warm contribute 0. The eviction assertion is unchanged and keeps its meaning. The resize also removes the rarer shapes where the same victim choice evicted keys 15 or 75 from the second cache, failing the get assertions just after.

Proof

After the resize no eviction can occur in the second phase by construction, and repeated runs of the test class pass.

Structural: after the resize the backing cache (200) exceeds everything the test can hold (127 put, at most 126 at once), so no eviction can occur in the second phase and the count is 0 by construction. The failure never reproduced locally on unmodified main in 17 runs. A standalone model of the put/get sequence against Caffeine 3.2.4 (the version this tree pins), 200,000 trials per configuration, fails the evictions check in 1,243 trials (0.62%) at capacity 100 and in none at 200; the phase-1 eviction arrived synchronously, before the triggering put returned, in every trial. Regression: 32 of 32 recorded runs pass on the original head 3c098439882, across seeds, repeats and CPU-load runs; 3 forced TestThinCache runs pass on this head after a comment-only correction (verified 2026-10-05 at head 77c019e1ff0). TestCaffeineCache 6/6 at head 3c098439882. Checks: tidy clean, Error Prone :solr:core:compileTestJava clean, :solr:core:check -x test green. No changelog: test-only fix.

Limits

The fix is main-only, the warming phase no longer exercises eviction pressure, and one phase-1 assertion still depends on the same victim choice.

Main only. Ten of the fourteen recorded occurrences were on branch_9x, where the test's metrics accessors have the older shape; the same fix shape applies but the accounting needs its own read first. Happy to prepare a branch_9x backport PR on request. The warming phase no longer exercises eviction pressure against a full shared backing; per-scope eviction attribution for that crosstalk depends on per-JVM hashes, so no exact assertion could cover it, and warm-under-pressure coverage belongs in a separate test designed around ranges or totals if maintainers want it. Separately, assertNull(lfuCache.get(1)) in phase 1 depends on the same victim choice (about 1 run in 10,000 in the model), with or without this change; this PR does not address that, and the test is not claimed to be deterministic.

AI assistance

AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.