Unverified Commit 7f0a7f0a authored by judeng's avatar judeng Committed by GitHub
Browse files

improve performance for scan command when matching data type (#12395)

Move the TYPE filtering to the scan callback so that avoided the
`lookupKey` operation. This is the follow-up to #12209 . In this thread
we introduced two breaking changes:
1. we will not attempt to do lazy expire (delete) a key that was
filtered by not matching the TYPE (like we already do for MATCH
pattern).
2. when the specified key TYPE filter is an unknown type, server will
reply a error immediately instead of doing a full scan that comes back
empty handed.
parent 3264deb2
......@@ -944,11 +944,10 @@ void scanCallback(void *privdata, const dictEntry *de) {
serverAssert(!((data->type != LLONG_MAX) && o));
/* Filter an element if it isn't the type we want. */
/* TODO: uncomment in redis 8.0
if (!o && data->type != LLONG_MAX) {
robj *rval = dictGetVal(de);
if (!objectTypeCompare(rval, data->type)) return;
}*/
}
/* Filter element if it does not match the pattern. */
void *keyStr = dictGetKey(de);
......@@ -1095,9 +1094,8 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
typename = c->argv[i+1]->ptr;
type = getObjectTypeByName(typename);
if (type == LLONG_MAX) {
/* TODO: uncomment in redis 8.0
addReplyErrorFormat(c, "unknown type name '%s'", typename);
return; */
return;
}
i+= 2;
} else if (!strcasecmp(c->argv[i]->ptr, "novalues")) {
......@@ -1285,16 +1283,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
while ((ln = listNext(&li))) {
sds key = listNodeValue(ln);
initStaticStringObject(kobj, key);
/* Filter an element if it isn't the type we want. */
/* TODO: remove this in redis 8.0 */
if (typename) {
robj* typecheck = lookupKeyReadWithFlags(c->db, &kobj, LOOKUP_NOTOUCH|LOOKUP_NONOTIFY);
if (!typecheck || !objectTypeCompare(typecheck, type)) {
listDelNode(keys, ln);
}
continue;
}
if (expireIfNeeded(c->db, &kobj, 0) != KEY_VALID) {
if (expireIfNeeded(c->db, &kobj, 0)) {
listDelNode(keys, ln);
}
}
......
......@@ -109,25 +109,9 @@ proc test_scan {type} {
after 2
# TODO: remove this in redis 8.0
set cur 0
set keys {}
while 1 {
set res [r scan $cur type "string1"]
set cur [lindex $res 0]
set k [lindex $res 1]
lappend keys {*}$k
if {$cur == 0} break
}
assert_equal 0 [llength $keys]
# make sure that expired key have been removed by scan command
assert_equal 1000 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
# TODO: uncomment in redis 8.0
#assert_error "*unknown type name*" {r scan 0 type "string1"}
assert_error "*unknown type name*" {r scan 0 type "string1"}
# expired key will be no touched by scan command
#assert_equal 1001 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
assert_equal 1001 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
r debug set-active-expire 1
} {OK} {needs:debug}
......@@ -191,11 +175,8 @@ proc test_scan {type} {
assert_equal 1000 [llength $keys]
# make sure that expired key have been removed by scan command
assert_equal 1000 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
# TODO: uncomment in redis 8.0
# make sure that only the expired key in the type match will been removed by scan command
#assert_equal 1001 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
assert_equal 1001 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
r debug set-active-expire 1
} {OK} {needs:debug}
......
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