Skip to content

Adapt HRANDFIELD to HFE feature#13348

Merged
moticless merged 10 commits intoredis:unstablefrom
moticless:hfe-adapt-hrandfield
Jun 24, 2024
Merged

Adapt HRANDFIELD to HFE feature#13348
moticless merged 10 commits intoredis:unstablefrom
moticless:hfe-adapt-hrandfield

Conversation

@moticless
Copy link
Collaborator

@moticless moticless commented Jun 17, 2024

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 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:

  • 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.

@moticless moticless requested a review from guybe7 June 17, 2024 13:19
@moticless moticless force-pushed the hfe-adapt-hrandfield branch from 76ef439 to b779bc4 Compare June 17, 2024 13:32
@moticless moticless merged commit e26ea35 into redis:unstable Jun 24, 2024
@moticless moticless deleted the hfe-adapt-hrandfield branch June 24, 2024 15:11
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.
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.

4 participants