Fix race condition in TestThinCache #3198
Merged
Conversation
This file contains 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
magibney
approved these changes
Feb 21, 2025
|
In the meantime I'll merge, since this is failing the tests occasionally. |
HoustonPutman
added a commit
that referenced
this pull request
Feb 21, 2025
(cherry picked from commit 85f50fa)
HoustonPutman
added a commit
that referenced
this pull request
Feb 21, 2025
(cherry picked from commit 85f50fa)
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.
Basically
onRemoval()has a much weaker guarantee than it is assuming right now.This method can be called asynchronously, after the key is already gone locally.
Or even after the new cache is created, and it doesn't have the value that is being removed in it's local cache.
This is a way to solve most of the issues, but one race condition still remains. (Not sure it's possible to solve)
If the cache evicts a key, then the user puts that key with its original value right afterwards, this code will still assume it should be removed. Since we don't have access to timings for removal and addition, there is no way to fix this. (I think).
But for the sake of our small test failures, this should be sufficient.