From 3032a99bd16649989a02858af966d68175f55d28 Mon Sep 17 00:00:00 2001
From: Binbin <binloveplay1314@qq.com>
Date: Wed, 20 Nov 2024 13:40:28 +0800
Subject: [PATCH] Make KEYS can visit expired key in import-source state

After #1185, a client in import-source state can visit expired
key both in read commands and write commands, this commit handle
keyIsExpired function to handle import-source state as well, so
KEYS can visit the expired key.

This is not particularly important, but it ensures the definition,
also doing some cleanup around the test, verified that the client
can indeed visit the expired key.

Signed-off-by: Binbin <binloveplay1314@qq.com>
---
 src/db.c              | 10 ++++++++--
 src/networking.c      |  2 +-
 tests/unit/expire.tcl | 23 ++++++++++++++---------
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/db.c b/src/db.c
index 10d4a04091..000a787890 100644
--- a/src/db.c
+++ b/src/db.c
@@ -388,7 +388,7 @@ robj *dbRandomKey(serverDb *db) {
             if (allvolatile && (server.primary_host || server.import_mode) && --maxtries == 0) {
                 /* If the DB is composed only of keys with an expire set,
                  * it could happen that all the keys are already logically
-                 * expired in the repilca, so the function cannot stop because
+                 * expired in the replica, so the function cannot stop because
                  * expireIfNeeded() is false, nor it can stop because
                  * dictGetFairRandomKey() returns NULL (there are keys to return).
                  * To prevent the infinite loop we do some tries, but if there
@@ -1798,7 +1798,13 @@ int keyIsExpiredWithDictIndex(serverDb *db, robj *key, int dict_index) {
 /* Check if the key is expired. */
 int keyIsExpired(serverDb *db, robj *key) {
     int dict_index = getKVStoreIndexForKey(key->ptr);
-    return keyIsExpiredWithDictIndex(db, key, dict_index);
+    if (!keyIsExpiredWithDictIndex(db, key, dict_index)) return 0;
+
+    /* See expireIfNeededWithDictIndex for more details. */
+    if (server.primary_host == NULL && server.import_mode) {
+        if (server.current_client && server.current_client->flag.import_source) return 0;
+    }
+    return 1;
 }
 
 keyStatus expireIfNeededWithDictIndex(serverDb *db, robj *key, int flags, int dict_index) {
diff --git a/src/networking.c b/src/networking.c
index 9558780f39..fb84cba6e8 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -3586,7 +3586,7 @@ void clientCommand(client *c) {
             "NO-TOUCH (ON|OFF)",
             "    Will not touch LRU/LFU stats when this mode is on.",
             "IMPORT-SOURCE (ON|OFF)",
-            "    Mark this connection as an import source if server.import_mode is true.",
+            "    Mark this connection as an import source if import-mode is enabled.",
             "    Sync tools can set their connections into 'import-source' state to visit",
             "    expired keys.",
             NULL};
diff --git a/tests/unit/expire.tcl b/tests/unit/expire.tcl
index fba425f62d..941acfad38 100644
--- a/tests/unit/expire.tcl
+++ b/tests/unit/expire.tcl
@@ -841,7 +841,7 @@ start_server {tags {"expire"}} {
 
         r set foo1 bar PX 1
         r set foo2 bar PX 1
-        after 100
+        after 10
 
         assert_equal [r dbsize] {2}
 
@@ -879,22 +879,27 @@ start_server {tags {"expire"}} {
         assert_equal [r debug set-active-expire 1] {OK}
     } {} {needs:debug}
 
-    test {RANDOMKEY can return expired key in import mode} {
+    test {Client can visit expired key in import-source state} {
         r flushall
 
         r config set import-mode yes
-        assert_equal [r client import-source on] {OK}
 
-        r set foo1 bar PX 1
+        r set foo1 1 PX 1
         after 10
 
-        set client [valkey [srv "host"] [srv "port"] 0 $::tls]
-        if {!$::singledb} {
-            $client select 9
-        }
-        assert_equal [$client ttl foo1] {-2}
+        # Normal clients cannot visit expired key.
+        assert_equal [r get foo1] {}
+        assert_equal [r ttl foo1] {-2}
+        assert_equal [r dbsize] 1
 
+        # Client can visit expired key when in import-source state.
+        assert_equal [r client import-source on] {OK}
+        assert_equal [r ttl foo1] {0}
+        assert_equal [r get foo1] {1}
+        assert_equal [r incr foo1] {2}
         assert_equal [r randomkey] {foo1}
+        assert_equal [r scan 0 match * count 10000] {0 foo1}
+        assert_equal [r keys *] {foo1}
 
         assert_equal [r client import-source off] {OK}
         r config set import-mode no