Unverified Commit f2f85ba3 authored by Filipe Oliveira (Redis)'s avatar Filipe Oliveira (Redis) Committed by GitHub
Browse files

Optimize SSCAN command in case of listpack or intset encoding: avoid the usage...

Optimize SSCAN command in case of listpack or intset encoding: avoid the usage of intermediate list. From 2N to N iterations (#13530)

On SSCAN, in case of listpack and intset encoding we actually reply the
entire set, and always reply with the cursor 0.

For those cases, we don't need to accumulate the replies in a list and
can completely avoid the overhead of list appending and then iterating
over the list again -- meaning we do N iterations instead of 2N
iterations over the SET and save intermediate memory as well.

Preliminary benchmarks, `SSCAN set:100 0`, showcased an improvement of
60% as visible bellow on a SET with 100 string elements (listpack
encoded).
parent c115c523
...@@ -1223,23 +1223,44 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) { ...@@ -1223,23 +1223,44 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
} }
} while (cursor && maxiterations-- && data.sampled < count); } while (cursor && maxiterations-- && data.sampled < count);
} else if (o->type == OBJ_SET) { } else if (o->type == OBJ_SET) {
unsigned long array_reply_len = 0;
void *replylen = NULL;
listRelease(keys);
char *str; char *str;
char buf[LONG_STR_SIZE]; char buf[LONG_STR_SIZE];
size_t len; size_t len;
int64_t llele; int64_t llele;
/* Reply to the client. */
addReplyArrayLen(c, 2);
/* Cursor is always 0 given we iterate over all set */
addReplyBulkLongLong(c,0);
/* If there is no pattern the length is the entire set size, otherwise we defer the reply size */
if (use_pattern)
replylen = addReplyDeferredLen(c);
else {
array_reply_len = setTypeSize(o);
addReplyArrayLen(c, array_reply_len);
}
setTypeIterator *si = setTypeInitIterator(o); setTypeIterator *si = setTypeInitIterator(o);
unsigned long cur_length = 0;
while (setTypeNext(si, &str, &len, &llele) != -1) { while (setTypeNext(si, &str, &len, &llele) != -1) {
if (str == NULL) { if (str == NULL) {
len = ll2string(buf, sizeof(buf), llele); len = ll2string(buf, sizeof(buf), llele);
} }
char *key = str ? str : buf; char *key = str ? str : buf;
if (use_pattern && !stringmatchlen(pat, sdslen(pat), key, len, 0)) { if (use_pattern && !stringmatchlen(pat, patlen, key, len, 0)) {
continue; continue;
} }
listAddNodeTail(keys, sdsnewlen(key, len)); addReplyBulkCBuffer(c, key, len);
cur_length++;
} }
setTypeReleaseIterator(si); setTypeReleaseIterator(si);
cursor = 0; if (use_pattern)
setDeferredArrayLen(c,replylen,cur_length);
else
serverAssert(cur_length == array_reply_len); /* fail on corrupt data */
return;
} else if ((o->type == OBJ_HASH || o->type == OBJ_ZSET) && } else if ((o->type == OBJ_HASH || o->type == OBJ_ZSET) &&
o->encoding == OBJ_ENCODING_LISTPACK) o->encoding == OBJ_ENCODING_LISTPACK)
{ {
......
...@@ -897,5 +897,17 @@ test {corrupt payload: fuzzer findings - set with invalid length causes smembers ...@@ -897,5 +897,17 @@ test {corrupt payload: fuzzer findings - set with invalid length causes smembers
} }
} }
test {corrupt payload: fuzzer findings - set with invalid length causes sscan to hang} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
# In the past, it generated a broken protocol and left the client hung in smembers
r config set sanitize-dump-payload no
assert_equal {OK} [r restore _set 0 "\x14\x16\x16\x00\x00\x00\x0c\x00\x81\x61\x02\x81\x62\x02\x81\x63\x02\x01\x01\x02\x01\x03\x01\xff\x0c\x00\x91\x00\x56\x73\xc1\x82\xd5\xbd" replace]
assert_encoding listpack _set
catch { r SSCAN _set 0 } err
assert_equal [count_log_message 0 "crashed by signal"] 0
assert_equal [count_log_message 0 "ASSERTION FAILED"] 1
}
}
} ;# tags } ;# tags
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