From f8ae991717f10c837c1a76b2954dae56ecb0e6bc Mon Sep 17 00:00:00 2001
From: guybe7 <guy.benoish@redislabs.com>
Date: Wed, 18 Nov 2020 10:16:21 +0100
Subject: [PATCH] EXISTS should not alter LRU, OBJECT should not reveal expired
 keys on replica (#8016)

The bug was introduced by #5021 which only attempted avoid EXIST on an
already expired key from returning 1 on a replica.

Before that commit, dbExists was used instead of
lookupKeyRead (which had an undesired effect to "touch" the LRU/LFU)

Other than that, this commit fixes OBJECT to also come empty handed on
expired keys in replica.

And DEBUG DIGEST-VALUE to behave like DEBUG OBJECT (get the data from
the key regardless of it's expired state)
---
 src/db.c                       | 25 ++++++++++++-------------
 src/debug.c                    |  6 +++++-
 src/object.c                   |  5 +----
 src/server.h                   |  1 +
 tests/unit/introspection-2.tcl |  3 ++-
 5 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/db.c b/src/db.c
index b8abb6877..fbbd63748 100644
--- a/src/db.c
+++ b/src/db.c
@@ -102,11 +102,8 @@ robj *lookupKeyReadWithFlags(redisDb *db, robj *key, int flags) {
         /* Key expired. If we are in the context of a master, expireIfNeeded()
          * returns 0 only when the key does not exist at all, so it's safe
          * to return NULL ASAP. */
-        if (server.masterhost == NULL) {
-            server.stat_keyspace_misses++;
-            notifyKeyspaceEvent(NOTIFY_KEY_MISS, "keymiss", key, db->id);
-            return NULL;
-        }
+        if (server.masterhost == NULL)
+            goto keymiss;
 
         /* However if we are in the context of a slave, expireIfNeeded() will
          * not really try to expire the key, it only returns information
@@ -125,19 +122,21 @@ robj *lookupKeyReadWithFlags(redisDb *db, robj *key, int flags) {
             server.current_client->cmd &&
             server.current_client->cmd->flags & CMD_READONLY)
         {
-            server.stat_keyspace_misses++;
-            notifyKeyspaceEvent(NOTIFY_KEY_MISS, "keymiss", key, db->id);
-            return NULL;
+            goto keymiss;
         }
     }
     val = lookupKey(db,key,flags);
-    if (val == NULL) {
+    if (val == NULL)
+        goto keymiss;
+    server.stat_keyspace_hits++;
+    return val;
+
+keymiss:
+    if (!(flags & LOOKUP_NONOTIFY)) {
         server.stat_keyspace_misses++;
         notifyKeyspaceEvent(NOTIFY_KEY_MISS, "keymiss", key, db->id);
     }
-    else
-        server.stat_keyspace_hits++;
-    return val;
+    return NULL;
 }
 
 /* Like lookupKeyReadWithFlags(), but does not use any flag, which is the
@@ -592,7 +591,7 @@ void existsCommand(client *c) {
     int j;
 
     for (j = 1; j < c->argc; j++) {
-        if (lookupKeyRead(c->db,c->argv[j])) count++;
+        if (lookupKeyReadWithFlags(c->db,c->argv[j],LOOKUP_NOTOUCH)) count++;
     }
     addReplyLongLong(c,count);
 }
diff --git a/src/debug.c b/src/debug.c
index 36cef5625..87e81eadb 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -644,7 +644,11 @@ NULL
         for (int j = 2; j < c->argc; j++) {
             unsigned char digest[20];
             memset(digest,0,20); /* Start with a clean result */
-            robj *o = lookupKeyReadWithFlags(c->db,c->argv[j],LOOKUP_NOTOUCH);
+
+            /* We don't use lookupKey because a debug command should
+             * work on logically expired keys */
+            dictEntry *de;
+            robj *o = ((de = dictFind(c->db->dict,c->argv[j]->ptr)) == NULL) ? NULL : dictGetVal(de);
             if (o) xorObjectDigest(c->db,c->argv[j],digest,o);
 
             sds d = sdsempty();
diff --git a/src/object.c b/src/object.c
index eef5e6d9e..d35ba0eaf 100644
--- a/src/object.c
+++ b/src/object.c
@@ -1229,10 +1229,7 @@ int objectSetLRUOrLFU(robj *val, long long lfu_freq, long long lru_idle,
 /* This is a helper function for the OBJECT command. We need to lookup keys
  * without any modification of LRU or other parameters. */
 robj *objectCommandLookup(client *c, robj *key) {
-    dictEntry *de;
-
-    if ((de = dictFind(c->db->dict,key->ptr)) == NULL) return NULL;
-    return (robj*) dictGetVal(de);
+    return lookupKeyReadWithFlags(c->db,key,LOOKUP_NOTOUCH|LOOKUP_NONOTIFY);
 }
 
 robj *objectCommandLookupOrReply(client *c, robj *key, robj *reply) {
diff --git a/src/server.h b/src/server.h
index 34c4e8360..53055897a 100644
--- a/src/server.h
+++ b/src/server.h
@@ -2179,6 +2179,7 @@ int objectSetLRUOrLFU(robj *val, long long lfu_freq, long long lru_idle,
                        long long lru_clock, int lru_multiplier);
 #define LOOKUP_NONE 0
 #define LOOKUP_NOTOUCH (1<<0)
+#define LOOKUP_NONOTIFY (1<<1)
 void dbAdd(redisDb *db, robj *key, robj *val);
 int dbAddRDBLoad(redisDb *db, sds key, robj *val);
 void dbOverwrite(redisDb *db, robj *key, robj *val);
diff --git a/tests/unit/introspection-2.tcl b/tests/unit/introspection-2.tcl
index 0968c2933..a34a15d67 100644
--- a/tests/unit/introspection-2.tcl
+++ b/tests/unit/introspection-2.tcl
@@ -3,11 +3,12 @@ proc cmdstat {cmd} {
 }
 
 start_server {tags {"introspection"}} {
-    test {TTL and TYPYE do not alter the last access time of a key} {
+    test {TTL, TYPE and EXISTS do not alter the last access time of a key} {
         r set foo bar
         after 3000
         r ttl foo
         r type foo
+        r exists foo
         assert {[r object idletime foo] >= 2}
     }