Unverified Commit 9a89e32a authored by Moti Cohen's avatar Moti Cohen Committed by GitHub
Browse files

HFE - Fix key ref by the hash on RENAME/MOVE/SWAPDB/RESTORE (#13539)

If the hash previously had HFEs (hash-fields with expiration) but later no longer
does, the key ref in the hash might become outdated after a MOVE, COPY,
RENAME or RESTORE operation. These commands maintain the key ref only
if HFEs are present. That is, we can only be sure that key ref is valid as long as the
hash has HFEs.
parent 610eb26c
......@@ -1134,6 +1134,20 @@ int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cm
dictEntry *de = dbFind(c->db, key->ptr);
serverAssert(de != NULL);
lpt->key = dictGetKey(de);
} else if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
listpackEx *lpt = o->ptr;
/* If the hash previously had HFEs but later no longer does, the key ref
* (lpt->key) in the hash might become outdated after a MOVE/COPY/RENAME/RESOTRE
* operation. These commands maintain the key ref only if HFEs are present.
* That is, we can only be sure that key ref is valid as long as it is not
* "trash". (TODO: dbFind() can be avoided. Instead need to extend the
* lookupKey*() to return dictEntry). */
if (lpt->meta.trash) {
dictEntry *de = dbFind(c->db, key->ptr);
serverAssert(de != NULL);
lpt->key = dictGetKey(de);
}
} else if (o->encoding == OBJ_ENCODING_HT) {
/* Take care dict has HFE metadata */
if (!isDictWithMetaHFE(ht)) {
......@@ -1151,6 +1165,18 @@ int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cm
m->key = dictGetKey(de); /* reference key in keyspace */
m->hfe = ebCreate(); /* Allocate HFE DS */
m->expireMeta.trash = 1; /* mark as trash (as long it wasn't ebAdd()) */
} else {
dictExpireMetadata *m = (dictExpireMetadata *) dictMetadata(ht);
/* If the hash previously had HFEs but later no longer does, the key ref
* (m->key) in the hash might become outdated after a MOVE/COPY/RENAME/RESTORE
* operation. These commands maintain the key ref only if HFEs are present.
* That is, we can only be sure that key ref is valid as long as it is not
* "trash". */
if (m->expireMeta.trash) {
dictEntry *de = dbFind(db, key->ptr);
serverAssert(de != NULL);
m->key = dictGetKey(de); /* reference key in keyspace */
}
}
}
......
......@@ -599,6 +599,20 @@ start_server {tags {"external:skip needs:debug"}} {
wait_for_condition 30 10 { [r exists myhash2] == 0 } else { fail "`myhash2` should be expired" }
}
test "Test RENAME hash that had HFEs but not during the rename ($type)" {
r del h1
r hset h1 f1 v1 f2 v2
r hpexpire h1 1 FIELDS 1 f1
after 20
r rename h1 h1_renamed
assert_equal [r exists h1] 0
assert_equal [r exists h1_renamed] 1
assert_equal [r hgetall h1_renamed] {f2 v2}
r hpexpire h1_renamed 1 FIELDS 1 f2
# Only active expire will delete the key
wait_for_condition 30 10 { [r exists h1_renamed] == 0 } else { fail "`h1_renamed` should be expired" }
}
test "MOVE to another DB hash with fields to be expired ($type)" {
r select 9
r flushall
......@@ -642,6 +656,20 @@ start_server {tags {"external:skip needs:debug"}} {
} {} {singledb:skip}
test "Test COPY hash that had HFEs but not during the copy ($type)" {
r del h1
r hset h1 f1 v1 f2 v2
r hpexpire h1 1 FIELDS 1 f1
after 20
r COPY h1 h1_copy
assert_equal [r exists h1] 1
assert_equal [r exists h1_copy] 1
assert_equal [r hgetall h1_copy] {f2 v2}
r hpexpire h1_copy 1 FIELDS 1 f2
# Only active expire will delete the key
wait_for_condition 30 10 { [r exists h1_copy] == 0 } else { fail "`h1_copy` should be expired" }
}
test "Test SWAPDB hash-fields to be expired ($type)" {
r select 9
r flushall
......@@ -663,6 +691,29 @@ start_server {tags {"external:skip needs:debug"}} {
wait_for_condition 20 10 { [r exists myhash] == 0 } else { fail "'myhash' should be expired" }
} {} {singledb:skip}
test "Test SWAPDB hash that had HFEs but not during the swap ($type)" {
r select 9
r flushall
r hset myhash f1 v1 f2 v2
r hpexpire myhash 1 NX FIELDS 1 f1
after 10
r swapdb 9 10
# Verify the key and its field doesn't exist in the source DB
assert_equal [r exists myhash] 0
assert_equal [r dbsize] 0
# Verify the key and its field exists in the target DB
r select 10
assert_equal [r hgetall myhash] {f2 v2}
assert_equal [r dbsize] 1
r hpexpire myhash 1 NX FIELDS 1 f2
# Eventually the field will be expired and the key will be deleted
wait_for_condition 20 10 { [r exists myhash] == 0 } else { fail "'myhash' should be expired" }
} {} {singledb:skip}
test "HMGET - returns empty entries if fields or hash expired ($type)" {
r debug set-active-expire 0
r del h1 h2
......@@ -731,6 +782,20 @@ start_server {tags {"external:skip needs:debug"}} {
assert_equal [r hexpiretime myhash FIELDS 3 a b c] {2524600800 2524600801 -1}
}
test {RESTORE hash that had in the past HFEs but not during the dump} {
r config set sanitize-dump-payload yes
r del myhash
r hmset myhash a 1 b 2 c 3
r hpexpire myhash 1 fields 1 a
after 10
set encoded [r dump myhash]
r del myhash
r restore myhash 0 $encoded
assert_equal [lsort [r hgetall myhash]] "2 3 b c"
r hpexpire myhash 1 fields 2 b c
wait_for_condition 30 10 { [r exists myhash] == 0 } else { fail "`myhash` should be expired" }
}
test {DUMP / RESTORE are able to serialize / unserialize a hash with TTL 0 for all fields} {
r config set sanitize-dump-payload yes
r del myhash
......
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