Repository navigation
Conversation
TestCaffeineCache.testSimple and TestThinCache.testSimple asserted that key 1 is evicted after 101 puts into a cache of size 100. Which entry is evicted is up to Caffeine's eviction policy, not insertion order, and it changed in Caffeine 3.3.0: both tests now fail on every branch_9x run. Assert that exactly one entry was evicted instead, and drop the get(1) lookup from the cumulative lookup count. TestThinCache also warms the second cache into the shared backing cache while it is still full. On Caffeine 3.3.0 the warmed entries lose to the existing ones and are evicted, so enlarge the backing before warming (the same change as SOLR-18506 / apache#5029).
|
🤖 AI text below 🤖 (reviewed by Nick Shanin) The change is safe on both Caffeine versions, so it can merge once the title and body are fixed. On fe797ea, TestCaffeineCache (6/6) and TestThinCache (2/2) pass with Caffeine 3.2.4 on main. The same 8 tests pass with 3.3.0, the version branch_9x uses since #4976. That bump is the cause: Caffeine 3.3.0 evicts a different entry for this sequence, so the tests should not assert which key is gone. The compare view shows the test-only change. Caffeine by branch: main (11.0) and branch_10x (10.2) use 3.2.4. branch_10_0 (10.0.x) uses 3.2.2. branch_9_11 (9.11.x) uses 3.2.4. branch_9x (9.12 dev) uses 3.3.0. Only branch_9x fails today. The same two victim assertions exist on every branch, so the next Caffeine bump to 3.3.0 would break the others too, and the test change should reach each of them before that bump. The email that prompted this cites #4967. That PR is SOLR-18134. The bump is #4976. Recommendation: Merge this to main, and have David Smiley do the branch_9x backport, since the diff does not apply there as-is (the test files differ). Before merge:
Once the title and body are fixed, this can merge to main. |
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
Thanks.
Nick is right -- refer to SOLR-18506 in the title.
|
I created separate PR #5207 for branch_9x packport and merged it already. So closing this one. |
Change related to failing test due to #4976 where Caffeine bumped up to 3.3.0, although main and 10_x still on older version 3.2.4, once we upgrade the fix will already in place.
https://develocity.apache.org/scans/tests?search.rootProjectNames=solr-root&search.tags=branch_9x&search.timeZoneId=Asia%2FCalcutta&tests.container=org.apache.solr.search.TestCaffeineCache&tests.test=testSimple
https://develocity.apache.org/scans/tests?search.rootProjectNames=solr-root&search.tags=branch_9x&search.timeZoneId=Asia%2FCalcutta&tests.container=org.apache.solr.search.TestThinCache&tests.test=testSimple