Skip to content

Conversation

@moticless
Copy link
Collaborator

@moticless moticless commented Jun 24, 2024

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 doesn't 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.

Please review also the modification I made to rewriteClientCommandArgument()
on a distinct PR to support deletion of argument.

@moticless moticless force-pushed the fix-hexpire-propagate branch from 77ceb30 to 4218881 Compare June 24, 2024 15:27
@moticless moticless marked this pull request as draft June 25, 2024 07:43
@moticless moticless changed the title Fix H(P)EXPIREAT command to propagate DEL as well Fix H(P)EXPIREAT command to propagate HDEL as well Jun 25, 2024
@moticless moticless marked this pull request as ready for review June 25, 2024 09:36
tezc
tezc previously approved these changes Jun 25, 2024
@moticless moticless merged commit 5eac99c into redis:unstable Jun 25, 2024
@moticless moticless deleted the fix-hexpire-propagate branch June 25, 2024 10:15
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants