Skip to content

Commit 72310d2

Browse files
oranagraenjoy-binbin
authored andcommitted
Fix issues with listpack encoded set (redis#11685)
PR redis#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 redis#11668). Fixed by copying logic from ZRANDMEMBER. note that both issues didn't exist in any redis release.
1 parent 09af8df commit 72310d2

File tree

2 files changed

+20
-7
lines changed

2 files changed

+20
-7
lines changed

src/object.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,8 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) {
10361036
if (samples) asize += (double)elesize/samples*dictSize(d);
10371037
} else if (o->encoding == OBJ_ENCODING_INTSET) {
10381038
asize = sizeof(*o)+zmalloc_size(o->ptr);
1039+
} else if (o->encoding == OBJ_ENCODING_LISTPACK) {
1040+
asize = sizeof(*o)+zmalloc_size(o->ptr);
10391041
} else {
10401042
serverPanic("Unknown set encoding");
10411043
}

src/t_set.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,11 @@ void spopCommand(client *c) {
969969
* implementation for more info. */
970970
#define SRANDMEMBER_SUB_STRATEGY_MUL 3
971971

972+
/* If client is trying to ask for a very large number of random elements,
973+
* queuing may consume an unlimited amount of memory, so we want to limit
974+
* the number of randoms per time. */
975+
#define SRANDFIELD_RANDOM_SAMPLE_LIMIT 1000
976+
972977
void srandmemberWithCountCommand(client *c) {
973978
long l;
974979
unsigned long count, size;
@@ -1010,13 +1015,19 @@ void srandmemberWithCountCommand(client *c) {
10101015

10111016
if (set->encoding == OBJ_ENCODING_LISTPACK && count > 1) {
10121017
/* Specialized case for listpack, traversing it only once. */
1013-
listpackEntry *entries = zmalloc(count * sizeof(listpackEntry));
1014-
lpRandomEntries(set->ptr, count, entries);
1015-
for (unsigned long i = 0; i < count; i++) {
1016-
if (entries[i].sval)
1017-
addReplyBulkCBuffer(c, entries[i].sval, entries[i].slen);
1018-
else
1019-
addReplyBulkLongLong(c, entries[i].lval);
1018+
unsigned long limit, sample_count;
1019+
limit = count > SRANDFIELD_RANDOM_SAMPLE_LIMIT ? SRANDFIELD_RANDOM_SAMPLE_LIMIT : count;
1020+
listpackEntry *entries = zmalloc(limit * sizeof(listpackEntry));
1021+
while (count) {
1022+
sample_count = count > limit ? limit : count;
1023+
count -= sample_count;
1024+
lpRandomEntries(set->ptr, sample_count, entries);
1025+
for (unsigned long i = 0; i < sample_count; i++) {
1026+
if (entries[i].sval)
1027+
addReplyBulkCBuffer(c, entries[i].sval, entries[i].slen);
1028+
else
1029+
addReplyBulkLongLong(c, entries[i].lval);
1030+
}
10201031
}
10211032
zfree(entries);
10221033
return;

0 commit comments

Comments
 (0)