Unverified Commit d0cc3de7 authored by Oran Agra's avatar Oran Agra Committed by GitHub
Browse files

Fix issues with listpack encoded set (#11685)

PR #11290 added listpack encoding for sets, but was missing two things:
1. Correct handling of MEMORY USAGE (leading to an assertion).
2. Had an uncontrolled scratch buffer size in SRANDMEMBER leading to
   OOM panic (reported in #11668). Fixed by copying logic from ZRANDMEMBER.

note that both issues didn't exist in any redis release.
parent cb1fff3c
...@@ -1036,6 +1036,8 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) { ...@@ -1036,6 +1036,8 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) {
if (samples) asize += (double)elesize/samples*dictSize(d); if (samples) asize += (double)elesize/samples*dictSize(d);
} else if (o->encoding == OBJ_ENCODING_INTSET) { } else if (o->encoding == OBJ_ENCODING_INTSET) {
asize = sizeof(*o)+zmalloc_size(o->ptr); asize = sizeof(*o)+zmalloc_size(o->ptr);
} else if (o->encoding == OBJ_ENCODING_LISTPACK) {
asize = sizeof(*o)+zmalloc_size(o->ptr);
} else { } else {
serverPanic("Unknown set encoding"); serverPanic("Unknown set encoding");
} }
......
...@@ -969,6 +969,11 @@ void spopCommand(client *c) { ...@@ -969,6 +969,11 @@ void spopCommand(client *c) {
* implementation for more info. */ * implementation for more info. */
#define SRANDMEMBER_SUB_STRATEGY_MUL 3 #define SRANDMEMBER_SUB_STRATEGY_MUL 3
/* If client is trying to ask for a very large number of random elements,
* queuing may consume an unlimited amount of memory, so we want to limit
* the number of randoms per time. */
#define SRANDFIELD_RANDOM_SAMPLE_LIMIT 1000
void srandmemberWithCountCommand(client *c) { void srandmemberWithCountCommand(client *c) {
long l; long l;
unsigned long count, size; unsigned long count, size;
...@@ -1010,13 +1015,19 @@ void srandmemberWithCountCommand(client *c) { ...@@ -1010,13 +1015,19 @@ void srandmemberWithCountCommand(client *c) {
if (set->encoding == OBJ_ENCODING_LISTPACK && count > 1) { if (set->encoding == OBJ_ENCODING_LISTPACK && count > 1) {
/* Specialized case for listpack, traversing it only once. */ /* Specialized case for listpack, traversing it only once. */
listpackEntry *entries = zmalloc(count * sizeof(listpackEntry)); unsigned long limit, sample_count;
lpRandomEntries(set->ptr, count, entries); limit = count > SRANDFIELD_RANDOM_SAMPLE_LIMIT ? SRANDFIELD_RANDOM_SAMPLE_LIMIT : count;
for (unsigned long i = 0; i < count; i++) { listpackEntry *entries = zmalloc(limit * sizeof(listpackEntry));
if (entries[i].sval) while (count) {
addReplyBulkCBuffer(c, entries[i].sval, entries[i].slen); sample_count = count > limit ? limit : count;
else count -= sample_count;
addReplyBulkLongLong(c, entries[i].lval); lpRandomEntries(set->ptr, sample_count, entries);
for (unsigned long i = 0; i < sample_count; i++) {
if (entries[i].sval)
addReplyBulkCBuffer(c, entries[i].sval, entries[i].slen);
else
addReplyBulkLongLong(c, entries[i].lval);
}
} }
zfree(entries); zfree(entries);
return; return;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment