Make KEYS can visit expired key in import-source state#1326
Merged
Conversation
After valkey-io#1185, a client in import-source state can visit expired key both in read commands and write commands, this commit handle keyIsExpired function to handle import-source state as well, so KEYS can visit the expired key. This is not particularly important, but it ensures the definition, also doing some cleanup around the test, verified that the client can indeed visit the expired key. Signed-off-by: Binbin <binloveplay1314@qq.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #1326 +/- ##
============================================
- Coverage 70.68% 70.57% -0.11%
============================================
Files 115 115
Lines 63177 63179 +2
============================================
- Hits 44657 44589 -68
- Misses 18520 18590 +70
|
Contributor
|
I guess an importing client will not use KEYS and SCAN, but I agree, it's good to ensure the contract that this client can read expired keys. |
lyq2333
reviewed
Nov 21, 2024
lyq2333
left a comment
Contributor
There was a problem hiding this comment.
Agree with @zuiderkwast. LGTM.
lyq2333
approved these changes
Nov 21, 2024
zuiderkwast
approved these changes
Nov 21, 2024
enjoy-binbin
added a commit
to enjoy-binbin/valkey
that referenced
this pull request
Nov 27, 2024
In valkey-io#1326 we make KEYS can visit expired key in import-source state by updating keyIsExpired to check for import mode. But after valkey-io#1205, we now use keyIsExpiredWithDictIndex to optimize and remove the redundant dict_index, and keyIsExpiredWithDictIndex does not handle this logic. In this commit, we handle keyIsExpiredWithDictIndex to make it check for import mode as well so that KEYS can visit the expired key. Signed-off-by: Binbin <binloveplay1314@qq.com>
enjoy-binbin
added a commit
that referenced
this pull request
Nov 28, 2024
) In #1326 we make KEYS can visit expired key in import-source state by updating keyIsExpired to check for import mode. But after #1205, we now use keyIsExpiredWithDictIndex to optimize and remove the redundant dict_index, and keyIsExpiredWithDictIndex does not handle this logic. In this commit, we handle keyIsExpiredWithDictIndex to make it check for import mode as well so that KEYS can visit the expired key. Signed-off-by: Binbin <binloveplay1314@qq.com>
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
3 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.
After #1185, a client in import-source state can visit expired
key both in read commands and write commands, this commit handle
keyIsExpired function to handle import-source state as well, so
KEYS can visit the expired key.
This is not particularly important, but it ensures the definition,
also doing some cleanup around the test, verified that the client
can indeed visit the expired key.