From f3bf8485d8c3f52cf05edc043f47c59e5c732305 Mon Sep 17 00:00:00 2001
From: Harkrishn Patro <harkrisp@amazon.com>
Date: Thu, 19 Oct 2023 03:58:32 -0700
Subject: [PATCH] Fix resize hash table dictionary iterator (#12660)

Dictionary iterator logic in the `tryResizeHashTables` method is picking the next
(incorrect) dictionary while the cursor is at a given slot. This could lead to some
dictionary/slot getting skipped from resizing.

Also stabilize the test.

problem introduced recently in #11695
---
 src/server.c          |  3 ++-
 src/server.h          |  1 +
 tests/unit/expire.tcl | 18 ++----------------
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/src/server.c b/src/server.c
index 29e9c71fd..912b5e148 100644
--- a/src/server.c
+++ b/src/server.c
@@ -608,8 +608,9 @@ void tryResizeHashTables(int dbid) {
     for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) {
         dbIterator *dbit = dbIteratorInitFromSlot(db, subdict, db->sub_dict[subdict].resize_cursor);
         for (int i = 0; i < CRON_DBS_PER_CALL; i++) {
-            dict *d = dbIteratorNextDict(dbit);
+            dict *d = dbGetDictFromIterator(dbit);
             slot = dbIteratorGetCurrentSlot(dbit);
+            dbIteratorNextDict(dbit);
             if (!d) break;
             if (htNeedsResize(d))
                 dictResize(d);
diff --git a/src/server.h b/src/server.h
index e6c45051d..852a45c00 100644
--- a/src/server.h
+++ b/src/server.h
@@ -2428,6 +2428,7 @@ typedef struct dbIterator dbIterator;
 dbIterator *dbIteratorInit(redisDb *db, dbKeyType keyType);
 dbIterator *dbIteratorInitFromSlot(redisDb *db, dbKeyType keyType, int slot);
 dict *dbIteratorNextDict(dbIterator *dbit);
+dict *dbGetDictFromIterator(dbIterator *dbit);
 int dbIteratorGetCurrentSlot(dbIterator *dbit);
 dictEntry *dbIteratorNext(dbIterator *iter);
 
diff --git a/tests/unit/expire.tcl b/tests/unit/expire.tcl
index e429c1ccc..672ba1b42 100644
--- a/tests/unit/expire.tcl
+++ b/tests/unit/expire.tcl
@@ -834,7 +834,7 @@ start_server {tags {"expire"}} {
     } {} {needs:debug}
 }
 
-start_cluster 1 0 {tags {"expire external:skip cluster"}} {
+start_cluster 1 0 {tags {"expire external:skip cluster slow"}} {
     test "expire scan should skip dictionaries with lot's of empty buckets" {
         # Collect two slots to help determine the expiry scan logic is able
         # to go past certain slots which aren't valid for scanning at the given point of time.
@@ -851,8 +851,6 @@ start_cluster 1 0 {tags {"expire external:skip cluster"}} {
         # hashslot(key) is 12539
         r psetex key 500 val
 
-        assert_equal 102 [r dbsize]
-
         # disable resizing
         r config set rdb-key-save-delay 10000000
         r bgsave
@@ -882,23 +880,11 @@ start_cluster 1 0 {tags {"expire external:skip cluster"}} {
             fail "bgsave did not stop in time."
         }
 
-        # Verify dict is under rehashing
-        set htstats [r debug HTSTATS 0]
-        assert_match {*rehashing target*} $htstats
-
         # put some data into slot 12182 and trigger the resize
         r psetex "{foo}0" 500 a
 
-        # Verify dict rehashing has completed
-        set htstats [r debug HTSTATS 0]
-        wait_for_condition 20 100 {
-            ![string match {*rehashing target*} $htstats]
-        } else {
-            fail "rehashing didn't complete"
-        }
-
         # Verify all keys have expired
-        wait_for_condition 20 100 {
+        wait_for_condition 200 100 {
             [r dbsize] eq 0
         } else {
             fail "Keys did not actively expire."