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

Optimize HSCAN/ZSCAN command in case of listpack encoding: avoid the usage of...


Optimize HSCAN/ZSCAN command in case of listpack encoding: avoid the usage of intermediate list (#13531)

Similar to #13530 , applied to HSCAN and ZSCAN in case of listpack
encoding.

**Preliminary benchmark results showcase an improvement of 108% on the
achievable ops/sec for ZSCAN and 65% for HSCAN**.

---------
Co-authored-by: default avatardebing.sun <debing.sun@redis.com>
parent ef3a5f58
...@@ -1267,27 +1267,51 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) { ...@@ -1267,27 +1267,51 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
unsigned char *p = lpFirst(o->ptr); unsigned char *p = lpFirst(o->ptr);
unsigned char *str; unsigned char *str;
int64_t len; int64_t len;
unsigned long array_reply_len = 0;
unsigned char intbuf[LP_INTBUF_SIZE]; unsigned char intbuf[LP_INTBUF_SIZE];
void *replylen = NULL;
listRelease(keys);
/* 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 = o->type == OBJ_HASH ? hashTypeLength(o, 0) : zsetLength(o);
if (!no_values) {
array_reply_len *= 2;
}
addReplyArrayLen(c, array_reply_len);
}
unsigned long cur_length = 0;
while(p) { while(p) {
str = lpGet(p, &len, intbuf); str = lpGet(p, &len, intbuf);
/* point to the value */ /* point to the value */
p = lpNext(o->ptr, p); p = lpNext(o->ptr, p);
if (use_pattern && !stringmatchlen(pat, sdslen(pat), (char *)str, len, 0)) { if (use_pattern && !stringmatchlen(pat, patlen, (char *)str, len, 0)) {
/* jump to the next key/val pair */ /* jump to the next key/val pair */
p = lpNext(o->ptr, p); p = lpNext(o->ptr, p);
continue; continue;
} }
/* add key object */ /* add key object */
listAddNodeTail(keys, sdsnewlen(str, len)); addReplyBulkCBuffer(c, str, len);
cur_length++;
/* add value object */ /* add value object */
if (!no_values) { if (!no_values) {
str = lpGet(p, &len, intbuf); str = lpGet(p, &len, intbuf);
listAddNodeTail(keys, sdsnewlen(str, len)); addReplyBulkCBuffer(c, str, len);
cur_length++;
} }
p = lpNext(o->ptr, p); p = lpNext(o->ptr, p);
} }
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->encoding == OBJ_ENCODING_LISTPACK_EX) { } else if (o->type == OBJ_HASH && o->encoding == OBJ_ENCODING_LISTPACK_EX) {
int64_t len; int64_t len;
long long expire_at; long long expire_at;
...@@ -1295,6 +1319,16 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) { ...@@ -1295,6 +1319,16 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
unsigned char *p = lpFirst(lp); unsigned char *p = lpFirst(lp);
unsigned char *str, *val; unsigned char *str, *val;
unsigned char intbuf[LP_INTBUF_SIZE]; unsigned char intbuf[LP_INTBUF_SIZE];
void *replylen = NULL;
listRelease(keys);
/* Reply to the client. */
addReplyArrayLen(c, 2);
/* Cursor is always 0 given we iterate over all set */
addReplyBulkLongLong(c,0);
/* In the case of OBJ_ENCODING_LISTPACK_EX we always defer the reply size given some fields might be expired */
replylen = addReplyDeferredLen(c);
unsigned long cur_length = 0;
while (p) { while (p) {
str = lpGet(p, &len, intbuf); str = lpGet(p, &len, intbuf);
...@@ -1305,7 +1339,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) { ...@@ -1305,7 +1339,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
serverAssert(p && lpGetIntegerValue(p, &expire_at)); serverAssert(p && lpGetIntegerValue(p, &expire_at));
if (hashTypeIsExpired(o, expire_at) || if (hashTypeIsExpired(o, expire_at) ||
(use_pattern && !stringmatchlen(pat, sdslen(pat), (char *)str, len, 0))) (use_pattern && !stringmatchlen(pat, patlen, (char *)str, len, 0)))
{ {
/* jump to the next key/val pair */ /* jump to the next key/val pair */
p = lpNext(lp, p); p = lpNext(lp, p);
...@@ -1313,15 +1347,18 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) { ...@@ -1313,15 +1347,18 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
} }
/* add key object */ /* add key object */
listAddNodeTail(keys, sdsnewlen(str, len)); addReplyBulkCBuffer(c, str, len);
cur_length++;
/* add value object */ /* add value object */
if (!no_values) { if (!no_values) {
str = lpGet(val, &len, intbuf); str = lpGet(val, &len, intbuf);
listAddNodeTail(keys, sdsnewlen(str, len)); addReplyBulkCBuffer(c, str, len);
cur_length++;
} }
p = lpNext(lp, p); p = lpNext(lp, p);
} }
cursor = 0; setDeferredArrayLen(c,replylen,cur_length);
return;
} else { } else {
serverPanic("Not handled encoding in SCAN."); serverPanic("Not handled encoding in SCAN.");
} }
......
...@@ -909,5 +909,27 @@ test {corrupt payload: fuzzer findings - set with invalid length causes sscan to ...@@ -909,5 +909,27 @@ test {corrupt payload: fuzzer findings - set with invalid length causes sscan to
} }
} }
test {corrupt payload: zset listpack encoded with invalid length causes zscan to hang} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload no
assert_equal {OK} [r restore _zset 0 "\x11\x16\x16\x00\x00\x00\x1a\x00\x81\x61\x02\x01\x01\x81\x62\x02\x02\x01\x81\x63\x02\x03\x01\xff\x0c\x00\x81\xa7\xcd\x31\x22\x6c\xef\xf7" replace]
assert_encoding listpack _zset
catch { r ZSCAN _zset 0 } err
assert_equal [count_log_message 0 "crashed by signal"] 0
assert_equal [count_log_message 0 "ASSERTION FAILED"] 1
}
}
test {corrupt payload: hash listpack encoded with invalid length causes hscan to hang} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload no
assert_equal {OK} [r restore _hash 0 "\x10\x17\x17\x00\x00\x00\x0e\x00\x82\x66\x31\x03\x82\x76\x31\x03\x82\x66\x32\x03\x82\x76\x32\x03\xff\x0c\x00\xf1\xc5\x36\x92\x29\x6a\x8c\xc5" replace]
assert_encoding listpack _hash
catch { r HSCAN _hash 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