fix(deps): Update com.github.ben-manes.caffeine:caffeine to v3.2.3 + adopt tests#11553
Open
snazy wants to merge 1 commit intoprojectnessie:mainfrom
Open
fix(deps): Update com.github.ben-manes.caffeine:caffeine to v3.2.3 + adopt tests#11553snazy wants to merge 1 commit intoprojectnessie:mainfrom
snazy wants to merge 1 commit intoprojectnessie:mainfrom
Conversation
1 task
dimas-b
reviewed
Oct 30, 2025
...torage/cache/src/test/java/org/projectnessie/versioned/storage/cache/TestCacheOvershoot.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| // The read of `Eviction` properties is "just" a volatile read since Caffeine 3.2.3 | ||
| // and trigger cleanups asynchronously. Before 3.2.3, cleanups happened synchronously. |
Member
There was a problem hiding this comment.
I'm ok with this upgrade, but I believe it will actually change "prod" code behaviour. Specifically, I expect the test I reported here to result in different memory allocation behaviour.
Do you think it's worth re-running that test?
Member
Author
There was a problem hiding this comment.
I think it doesn't hurt re-running that test.
Member
Author
|
Um - so |
…adopt tests The read of `Eviction` properties is "just" a volatile read since Caffeine 3.2.3 and trigger cleanups asynchronously. Before 3.2.3, cleanups happened synchronously. This change breaks the initially present assertions of this test, but not the functionality of the production code. There is sadly no fix possible with Caffeine since 3.2.3. See [this reply](ben-manes/caffeine#1897 (comment)). The workaround I came up with is to change the logic a bit: * If the cache-weight is less than the capacity: just put the entry to the cache, no "special handling". * Otherwise do the following while holding an exclusive lock: * Explicitly trigger cache cleanup * If the cache-weight is less than the admitted capacity (overshooting), put the entry into the cache. * Else, reject and update meters accordingly. We have to allow the "overshooting" to happen to enable (and trigger) the cache cleanup. Otherwise, cleanup would never happen ...
snazy
force-pushed
the
caffeine-3.2.3
branch
from
5c83df4 to
72c69d6
Compare
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The read of
Evictionproperties is "just" a volatile read since Caffeine 3.2.3 and trigger cleanups asynchronously. Before 3.2.3, cleanups happened synchronously. This change breaks the initially present assertions of this test, but not the functionality of the production code. See ben-manes/caffeine#1897