Skip to content
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

ZRANDMEMBER only ever returns first n elements in sorted set #2850

Closed
la-mar opened this issue Apr 5, 2024 · 3 comments · Fixed by #2994
Closed

ZRANDMEMBER only ever returns first n elements in sorted set #2850

la-mar opened this issue Apr 5, 2024 · 3 comments · Fixed by #2994
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed minor nice to have enhancement Next Up task that is ready to be worked on and should be added to working queue

Comments

@la-mar
Copy link

la-mar commented Apr 5, 2024

Describe the bug
ZRANDMEMBER should return random elements from the specified sorted set, instead, it only returns the lowest scoring element(s).

To Reproduce
Steps to reproduce the behavior:

  1. Insert some elements into a sorted set: redis-cli zadd x 2 b 1 a 4 d 3 c
  2. Get a random member multiple times: seq 25 | xargs -I {} redis-cli ZRANDMEMBER x 1
  3. The output will always be "a".

Expected behavior
I expect to get a pseudo random element for each invocation of ZRANDMEMBER.

The same steps above produce the expected randomized response when run against a standard redis docker image:

# seq 25 | xargs -I {} redis-cli ZRANDMEMBER x 1
1) "a"
1) "a"
1) "d"
1) "b"
1) "d"
1) "b"
1) "d"
1) "b"
1) "a"
1) "c"
1) "b"
1) "a"
1) "b"
1) "d"
1) "a"
1) "b"
1) "d"
1) "a"
1) "b"
1) "d"
1) "d"
1) "b"
1) "b"
1) "d"
1) "a"

Environment (please complete the following information):

  • OS: ubuntu 20.04
  • Kernel: Linux 7e19e630bc09 6.4.16-linuxkit finalize blpop algorithm #1 SMP PREEMPT Fri Nov 10 14:49:23 UTC 2023 aarch64 aarch64 aarch64 GNU/Linux
  • Containerized?: Docker
  • Dragonfly Version: v1.15.0

Reproducible Code Snippet

redis-cli zadd x 2 b 1 a 4 d 3 c
seq 25 | xargs -I {} redis-cli ZRANDMEMBER x 1

Additional context
The dragonfly website says it is fully compatible with Redis' implementation of ZRANDMEMBER, so it appears to be a bug instead of an intentional partial implementation.

@la-mar la-mar added the bug Something isn't working label Apr 5, 2024
@romange
Copy link
Collaborator

romange commented Apr 5, 2024

Thanks for reporting this. Out of curiosity how do you use this command? I am asking because I have not seen much traction with RAND commands of Redis API.

@la-mar
Copy link
Author

la-mar commented Apr 6, 2024

Sure thing! Thanks for the quick response.

In this case, I am keeping global set of unique identifiers and the timestamp they were last updated in a handful of other data structures that all tie back to that identifier to track when they can be safely expired. The rand usage is probably somewhat niche - I use it to facilitate a couple of kinds of load testing of the datastore and the soft real-time API that sits in front of it. One such scenario is generating a constant read load in a production setting alongside normal traffic. The randomness is useful for generating synthetic read load that is dynamic to data that is known to be in the system at any given time.

There are, of course, a thousand other ways to skin that cat, but I've found that this one to be one of the simplest and effective for the use cases (real-time, high data volume) I tend to have.

@romange romange added Next Up task that is ready to be worked on and should be added to working queue minor nice to have enhancement labels Apr 6, 2024
@romange romange added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 28, 2024
@BagritsevichStepan
Copy link
Contributor

I would like to add that SRANDMEMBER and SPOP have the same problem. Both commands return/remove only the lowest scoring element(s).

SRANDMEMBER

The steps to reproduce the behavior of the SRANDMEMBER command are the same as in the case of ZRANDMEMBER:

  1. Inserting elements into a set: redis-cli SADD myset one two three four five six seven eight nine ten
  2. Сalling SRANDMEMBER multiple times: seq 20 | xargs -I {} redis-cli SRANDMEMBER myset

The result is always "eight".

SPOP

The steps to reproduce the SPOP behavior are more tricky. After each deletion of an element, we must return it back to the set in order to keep the same set of elements:

  1. Insert elements into a set: redis-cli SADD myset one two three four five six seven eight nine ten

  2. Remove the element and return it back multiple times: for i in {1..30}; do redis-cli SPOP myset | tee -a removed_members.txt | xargs -I {} redis-cli SADD myset {}; done

    • Notice: between running SPOP and SADD, we write the SPOP result to the file removed_members.txt. As a result, in removed_elements.txt we can see which elements were deleted during the SPOP commands.

In removed_element.txt we can see that "eight" was deleted each time.

Expected behavior

Redis always removes a pseudo-random element. So, the result may differ between runs:

$ cat removed_members.txt
ten
nine
four
five
eight
eight
one
nine
two
nine
three
five
four
nine
eight
five
four
two
six
two
two
ten
eight
two
five
ten
two
four
six
nine

BagritsevichStepan added a commit to BagritsevichStepan/dragonfly that referenced this issue May 1, 2024
BagritsevichStepan added a commit to BagritsevichStepan/dragonfly that referenced this issue May 1, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 2, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 2, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 3, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 4, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 4, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 5, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 5, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 5, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 5, 2024
BagritsevichStepan pushed a commit to BagritsevichStepan/dragonfly that referenced this issue May 5, 2024
adiholden pushed a commit that referenced this issue May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed minor nice to have enhancement Next Up task that is ready to be worked on and should be added to working queue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants