more test fixes and fixing listpack to dict conversion not added to g…

…lobal HFE DS
redis · May 16, 2024 · d9d4440 · d9d4440
1 parent 37cc9eb
commit d9d4440
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 7 deletions.
diff --git a/src/rdb.c b/src/rdb.c
@@ -2703,6 +2703,20 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int rdbflags,
                     (hashTypeLength(o, 0) > server.hash_max_listpack_entries))  /* TODO: each field length is not verified against server.hash_max_listpack_value */
                 {
                     hashTypeConvert(o, OBJ_ENCODING_HT, &db->hexpires);
+                    /*
+                     * hashTypeAddToExpires is presumably called from within
+                     * the convert function (from listpackEx to dict), BUT,
+                     * this call depends on the lpt->meta field to be updated,
+                     * which is not the case here as hashTypeAddToExpires was
+                     * not yet called for the listpack (which is what updating
+                     * its meta).
+                     * Instead, this "manual" call is added here.
+                     * Another approach would be to have the conversion function
+                     * find the minExpire by itself when iterating on the listpack
+                     * instead of relying on the meta and use this value for the
+                     * final ebAdd call.
+                     */
+                    hashTypeAddToExpires(db, key, o, minExpire);
                 } else if (rdbtype == RDB_TYPE_HASH_LISTPACK_EX) {
                     /* connect the listpack to the DB-global expiry data structure */
                     if ((minExpire != EB_EXPIRE_TIME_INVALID) && (db != NULL)) { /* DB can be NULL when checking rdb */

diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl
@@ -535,13 +535,14 @@ test "save listpack, load dict" {
 
         # change configuration and reload - result should be dict-encoded key
         r config set hash-max-listpack-entries 0
-        r debug reload
+        r debug reload nosave
 
         # first verify d was not expired during load (no expiry when loading
         # a hash that was saved listpack-encoded)
         assert_equal [s rdb_last_load_keys_loaded] 1
         assert_equal [s rdb_last_load_hash_fields_expired] 0
 
+        # d should be lazy expired in hgetall
         assert_equal [lsort [r hgetall key]] "1 2 3 a b c"
         assert_match "*encoding:hashtable*" [r debug object key]
     }
@@ -557,15 +558,15 @@ test "save dict, load listpack" {
 
         r HMSET key a 1 b 2 c 3 d 4
         assert_match "*encoding:hashtable*" [r debug object key]
-        r HPEXPIRE key 2000 1 d
+        r HPEXPIRE key 200 1 d
         r save
 
-        # sleep 2001 ms to make sure 'd' will expire during reload
-        after 2001
+        # sleep 201 ms to make sure 'd' will expire during reload
+        after 201
 
         # change configuration and reload - result should be LP-encoded key
         r config set hash-max-listpack-entries 512
-        r debug reload
+        r debug reload nosave
 
         # verify d was expired during load
         assert_equal [s rdb_last_load_keys_loaded] 1
@@ -591,7 +592,7 @@ foreach {type lp_entries} {listpack 512 dict 0} {
             r HPEXPIRE key 200 2 c d
 
             r save
-            r debug reload
+            r debug reload nosave
 
             # wait at most 2 secs to make sure 'c' and 'd' will active-expire
             wait_for_condition 20 100 {
@@ -625,7 +626,7 @@ foreach {type lp_entries} {listpack 512 dict 0} {
             r HPEXPIRE key 200 2 c d
 
             r save
-            r debug reload
+            r debug reload nosave
 
             # sleep 500 msec to make sure 'c' and 'd' will lazy-expire when calling hgetall
             after 500