-
Notifications
You must be signed in to change notification settings - Fork 24.4k
Fix H(P)EXPIREAT command to propagate HDEL as well #13364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
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
77ceb30 to
4218881
Compare
tezc
reviewed
Jun 24, 2024
tezc
reviewed
Jun 24, 2024
tezc
reviewed
Jun 24, 2024
tezc
reviewed
Jun 24, 2024
sundb
reviewed
Jun 25, 2024
tezc
reviewed
Jun 25, 2024
tezc
reviewed
Jun 25, 2024
tezc
reviewed
Jun 25, 2024
tezc
reviewed
Jun 25, 2024
tezc
previously approved these changes
Jun 25, 2024
tezc
approved these changes
Jun 25, 2024
funny-dog
pushed a commit
to funny-dog/redis
that referenced
this pull request
Sep 17, 2025
H(P)EXPIREAT command might delete fields in case the absolute time is in the past. Those HDELs need to be propagated as well. In general, as we need to propagate H(P)EXPIRE(AT) command to the replica, each field that is mentioned in the command should be categorized into one of the four options: 1. Managed to update field’s expiration time - propagate it to replica as part of the HPEXPIREAT command. 2. Deleted the field because the time is in the past - propagate also HDEL command to delete the field and remove the field from the propagated HPEXPIREAT. 3. Condition not met for the field - Remove the field from the propagated HPEXPIREAT command. 4. Field does not exists - Remove the field from the propagated HPEXPIREAT command. If none of the provided fields match option number 1, then avoid also propagating the HPEXPIREAT command to the replica. This approach is aligned with the EXPIRE command: If a given key has already expired, then DEL will be propagated instead of EXPIRE command. If condition not met, then command will be rejected. Otherwise, EXPIRE command will be propagated for given key.
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.
H(P)EXPIREAT command might delete fields in case the absolute time is in the past.
Those HDELs need to be propagated as well.
In general, as we need to propagate H(P)EXPIRE(AT) command to the replica, each field that is
mentioned in the command should be categorized into one of the four options:
part of the HPEXPIREAT command.
to delete the field and remove the field from the propagated HPEXPIREAT.
HPEXPIREAT command.
If none of the provided fields match option number 1, then avoid also propagating the
HPEXPIREAT command to the replica.
This approach is aligned with the EXPIRE command:
If a given key has already expired, then DEL will be propagated instead of
EXPIRE command. If condition not met, then command will be rejected.
Otherwise, EXPIRE command will be propagated for given key.
Please review also the modification I made to rewriteClientCommandArgument()
on a distinct PR to support deletion of argument.