Adapt HRANDFIELD to HFE feature#13348
Merged
moticless merged 10 commits intoredis:unstablefrom Jun 24, 2024
Merged
Conversation
76ef439 to
b779bc4
Compare
sundb
reviewed
Jun 17, 2024
guybe7
reviewed
Jun 18, 2024
guybe7
reviewed
Jun 18, 2024
guybe7
reviewed
Jun 18, 2024
guybe7
reviewed
Jun 18, 2024
tezc
reviewed
Jun 24, 2024
tezc
reviewed
Jun 24, 2024
tezc
reviewed
Jun 24, 2024
tezc
reviewed
Jun 24, 2024
tezc
approved these changes
Jun 24, 2024
funny-dog
pushed a commit
to funny-dog/redis
that referenced
this pull request
Sep 17, 2025
Considerations for the selected imp of HRANDFIELD & HFE feature: HRANDFIELD might access any of the fields in the hash as some of them might be expired. And so the Implementation of HRANDFIELD along with HFEs might be one of the two options: 1. Expire hash-fields before diving into handling HRANDFIELD. 2. Refine HRANDFIELD cases to deal with expired fields. Regarding the first option, as reference, the command RANDOMKEY also declareson O(1) complexity, yet might be stuck on a very long (but not infinite) loop trying to find non-expired keys. Furthermore RANDOMKEY also evicts expired keys along the way even though it is categorized as a read-only command. Note that the case of HRANDFIELD is more lightweight versus RANDOMKEY since HFEs have much more effective and aggressive active-expiration for fields behind. The second option introduces additional implementation complexity to HRANDFIELD. We could further refine HRANDFIELD cases to differentiate between scenarios with many expired fields versus few expired fields, and adjust based on the percentage of expired fields. However, this approach could still lead to long loops or necessitate expiring fields before selecting them. For the “lightweight” cases it is also expected to have a lightweight expiration. Considering the pros and cons, and the fact that HRANDFIELD is an infrequent command (particularly with HFEs) and the fact we have effective active-expiration behind for hash-fields, it is better to keep it simple and choose option number 1. Other changes: * Don't mark command dirty by internal hashTypeExpire(). It causes to read only command of HRANDFIELD to be accidently propagated (This flag should be indicated at higher level, by the command functions). * Align `hashTypeExpireIfNeeded()` and `hashTypeGetValue()` to be more aligned with `expireIfNeeded()` logic of keyspace.
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.
Considerations for the selected imp of HRANDFIELD & HFE feature:
HRANDFIELD might access any of the fields in the hash as some of them might
be expired. And so the Implementation of HRANDFIELD along with HFEs
might be one of the two options:
Regarding the first option, as reference, the command RANDOMKEY also declares
on O(1) complexity, yet might be stuck on a very long (but not infinite) loop
trying to find non-expired keys. Furthermore RANDOMKEY also evicts expired keys
along the way even though it is categorized as a read-only command. Note that
the case of HRANDFIELD is more lightweight versus RANDOMKEY since HFEs have
much more effective and aggressive active-expiration for fields behind.
The second option introduces additional implementation complexity to HRANDFIELD.
We could further refine HRANDFIELD cases to differentiate between scenarios
with many expired fields versus few expired fields, and adjust based on the
percentage of expired fields. However, this approach could still lead to long
loops or necessitate expiring fields before selecting them. For the “lightweight”
cases it is also expected to have a lightweight expiration.
Considering the pros and cons, and the fact that HRANDFIELD is an infrequent
command (particularly with HFEs) and the fact we have effective active-expiration
behind for hash-fields, it is better to keep it simple and choose option number 1.
Other changes:
hashTypeExpireIfNeeded()andhashTypeGetValue()to be more aligned withexpireIfNeeded()logic of keyspace.