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

CLUSTER GETKEYSINSLOT unnecessarily allocates memory #3911

Closed
ghik opened this issue Mar 31, 2017 · 2 comments
Closed

CLUSTER GETKEYSINSLOT unnecessarily allocates memory #3911

ghik opened this issue Mar 31, 2017 · 2 comments
Labels
Milestone

Comments

@ghik
Copy link

ghik commented Mar 31, 2017

In my tests I wanted to invoke a command which would return all keys from some slot and in order to do this, I passed Int.MaxValue (231-1) as COUNT argument to CLUSTER GETKEYSINSLOT command. The result was Redis crash with an OOM.

It seems that Redis blindly allocates amount of memory proportional to COUNT (https://github.com/antirez/redis/blob/unstable/src/cluster.c#L4267) even though the actual number of keys in the slot may be much smaller.

@antirez antirez added this to the 4.0 final milestone Mar 31, 2017
@antirez
Copy link
Contributor

antirez commented Mar 31, 2017

Hello, yes that's bad... The goal of the command is to return just a few keys per invocation since the command just targets Redis Cluster migrations and everything else should be performed via SCAN, however this is not ok. Btw I'm totally reimplementing this part of Redis as we speak so this will be fixed soon. Thanks.

@antirez
Copy link
Contributor

antirez commented Jul 11, 2017

Thanks, fixed.

@antirez antirez closed this as completed Jul 11, 2017
antirez added a commit that referenced this issue Jul 11, 2017
antirez added a commit that referenced this issue Jul 14, 2017
GitHubMota pushed a commit to GitHubMota/redis that referenced this issue Jul 25, 2017
JackieXie168 pushed a commit to JackieXie168/redis that referenced this issue Aug 20, 2017
JunHe77 pushed a commit to JunHe77/redis that referenced this issue Dec 1, 2017
JackieXie168 pushed a commit to JackieXie168/redis that referenced this issue Jan 13, 2018
pulllock pushed a commit to pulllock/redis that referenced this issue Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants