fix: computeIfAbsent should not update write timestamp for existing entries#8301
Open
daguimu wants to merge 1 commit intogoogle:masterfrom
Open
fix: computeIfAbsent should not update write timestamp for existing entries#8301daguimu wants to merge 1 commit intogoogle:masterfrom
daguimu wants to merge 1 commit intogoogle:masterfrom
Conversation
…ntries When computeIfAbsent is called on an existing key, the value is not changed, but recordWrite was called which incorrectly reset the write timestamp. This caused expireAfterWrite caches to behave like expireAfterAccess, as repeated computeIfAbsent calls would extend the entry's lifetime indefinitely. The fix avoids calling recordWrite when the computed value is the same reference as the existing value. Instead, it updates only the access time and re-adds the entry to both access and write queues without resetting the write timestamp. Fixes google#3700
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
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.
Problem
LocalCache.computeIfAbsent()incorrectly resets the write timestamp when called on an existing key, even though the value is not changed. This causesexpireAfterWritecaches to behave likeexpireAfterAccesscaches — entries never expire as long as they are accessed viacomputeIfAbsent()at intervals shorter than the write expiry duration.This is particularly problematic for Spring Boot applications using
@Cacheablewith synchronized access, which delegates tocomputeIfAbsent()internally.Root Cause
computeIfAbsent()delegates toSegment.compute(). When the computed value is the same reference as the existing value (newValue == valueReference.get()), the code path atLocalCache.java:2278calledrecordWrite(e, 0, now), which resets the write timestamp viaentry.setWriteTime(now). This incorrectly extends theexpireAfterWritedeadline.Fix
Replaced the
recordWrite(e, 0, now)call with inline logic that:expireAfterAccesscorrectness)accessQueueandwriteQueue(required because they were removed earlier in thecompute()method at lines 2251-2252)expireAfterWritedeadlineTests Added
testComputeIfAbsent_existingEntryWriteTimeNotChanged()— verifiesgetWriteTime()is unchanged aftercomputeIfAbsenton existing keytestComputeIfAbsent_existingEntryExpiresAfterWrite()— verifies entry expires after write duration even with repeatedcomputeIfAbsentcallstestCompute_returningNewValueUpdatesWriteTime()— verifies thatcompute()returning a new value still correctly updates write timeImpact
Only affects the code path in
Segment.compute()where the computed value is the same reference as the existing value. This path is exercised bycomputeIfAbsent()on existing keys, and also bycompute()/computeIfPresent()/merge()when their function returns the existing value unchanged. The new-value and removal paths are unaffected.Fixes #3700