From e750c619b254576e88c5f3aeff73eca6b39a64de Mon Sep 17 00:00:00 2001
From: "debing.sun" <debing.sun@redis.com>
Date: Wed, 31 Jul 2024 08:15:39 +0800
Subject: [PATCH] Fix some test failures caused by key being deleted due to
 premature expiration (#13453)

1. Fix fuzzer test failure when the key was deleted due to expiration
before sending random traffic for the key.

After HFE, when all fields in a hash are expired, the hash might be
deleted due to expiration.

If the key was expired in the mid of `RESTORE` command and sending rand
trafic, `fuzzer` test will fail in the following code because the 'TYPE
key' will return `none` and then throw an exception because it cannot be
found in `$commands`

https://github.com/redis/redis/blob/94b9072e44af0bae8cfe2de5cfa4af7b8e399759/tests/support/util.tcl#L712-L713

This PR adds a `None` check for the reply of `KEY TYPE` command and adds
a print of `err` to avoid false positives in the future.

failed CI:
https://github.com/redis/redis/actions/runs/10127334121/job/28004985388

2. Fix the issue where key was deleted due to expiration before the
`scan.scan_key` command could execute, caused by premature enabling of
`set-active-expire`.

failed CI:
https://github.com/redis/redis/actions/runs/10153722715/job/28077610552

---------

Co-authored-by: oranagra <oran@redislabs.com>
---
 tests/integration/corrupt-dump-fuzzer.tcl | 15 +++++++++++++--
 tests/support/util.tcl                    |  3 +--
 tests/unit/moduleapi/scan.tcl             |  4 ++--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tests/integration/corrupt-dump-fuzzer.tcl b/tests/integration/corrupt-dump-fuzzer.tcl
index 84495886c..ed6a15bd1 100644
--- a/tests/integration/corrupt-dump-fuzzer.tcl
+++ b/tests/integration/corrupt-dump-fuzzer.tcl
@@ -122,6 +122,7 @@ foreach sanitize_dump {no yes} {
                 set restore_failed false
                 set report_and_restart false
                 set sent {}
+                set expired_subkeys [s expired_subkeys]
                 # RESTORE can fail, but hopefully not terminate
                 if { [catch { r restore "_$k" 0 $dump REPLACE } err] } {
                     set restore_failed true
@@ -145,7 +146,17 @@ foreach sanitize_dump {no yes} {
                     # if RESTORE didn't fail or terminate, run some random traffic on the new key
                     incr stat_successful_restore
                     if { [ catch {
-                        set sent [generate_fuzzy_traffic_on_key "_$k" 1] ;# traffic for 1 second
+                        set type [r type "_$k"]
+                        if {$type eq {none}} {
+                            # The key has been removed due to expiration.
+                            # Ensure the server didn't terminate during expiration and verify
+                            # expire stats to confirm the key was removed due to expiration.
+                            r ping
+                            assert_morethan [s expired_subkeys] $expired_subkeys
+                        } else {
+                            set sent [generate_fuzzy_traffic_on_key "_$k" $type 1] ;# traffic for 1 second
+                        }
+
                         incr stat_traffic_commands_sent [llength $sent]
                         r del "_$k" ;# in case the server terminated, here's where we'll detect it.
                         if {$dbsize != [r dbsize]} {
@@ -182,7 +193,7 @@ foreach sanitize_dump {no yes} {
                                 dump_server_log $srv
                             }
 
-                            puts "Server crashed (by signal: $by_signal), with payload: $printable_dump"
+                            puts "Server crashed (by signal: $by_signal, err: $err), with payload: $printable_dump"
                             set print_commands true
                         }
                     }
diff --git a/tests/support/util.tcl b/tests/support/util.tcl
index 858fc5a55..f374c3dc9 100644
--- a/tests/support/util.tcl
+++ b/tests/support/util.tcl
@@ -698,7 +698,7 @@ proc latencyrstat_percentiles {cmd r} {
     }
 }
 
-proc generate_fuzzy_traffic_on_key {key duration} {
+proc generate_fuzzy_traffic_on_key {key type duration} {
     # Commands per type, blocking commands removed
     # TODO: extract these from COMMAND DOCS, and improve to include other types
     set string_commands {APPEND BITCOUNT BITFIELD BITOP BITPOS DECR DECRBY GET GETBIT GETRANGE GETSET INCR INCRBY INCRBYFLOAT MGET MSET MSETNX PSETEX SET SETBIT SETEX SETNX SETRANGE LCS STRLEN}
@@ -709,7 +709,6 @@ proc generate_fuzzy_traffic_on_key {key duration} {
     set stream_commands {XACK XADD XCLAIM XDEL XGROUP XINFO XLEN XPENDING XRANGE XREAD XREADGROUP XREVRANGE XTRIM}
     set commands [dict create string $string_commands hash $hash_commands zset $zset_commands list $list_commands set $set_commands stream $stream_commands]
 
-    set type [r type $key]
     set cmds [dict get $commands $type]
     set start_time [clock seconds]
     set sent {}
diff --git a/tests/unit/moduleapi/scan.tcl b/tests/unit/moduleapi/scan.tcl
index 2f0127267..6c26de443 100644
--- a/tests/unit/moduleapi/scan.tcl
+++ b/tests/unit/moduleapi/scan.tcl
@@ -61,9 +61,9 @@ start_server {tags {"modules"}} {
         r hmset hh f1 v1 f2 v2 f3 v3
         r hpexpire hh 1 fields 3 f1 f2 f3
         after 10
+        assert_equal [r scan.scan_key hh] {}
         r debug set-active-expire 1
-        lsort [r scan.scan_key hh]
-    } {} {needs:debug}
+    } {OK} {needs:debug}
 
     test {Module scan zset listpack} {
         r zadd zz 1 f1 2 f2