prevent client tracking from causing feedback loop in performEvictions (#8100)

When client tracking is enabled signalModifiedKey can increase memory usage,
this can cause the loop in performEvictions to keep running since it was measuring
the memory usage impact of signalModifiedKey.

The section that measures the memory impact of the eviction should be just on dbDelete,
excluding keyspace notification, client tracking, and propagation to AOF and replicas.

This resolves part of the problem described in #8069
p.s. fix took 1 minute, test took about 3 hours to write.

(cherry picked from commit c4fdf09c0584a3cee32b92f01b7958c72776aedc)

src/evict.c

@@ -562,6 +562,8 @@ int freeMemoryIfNeeded(void) {
              * we are freeing removing the key, but we can't account for
              * that otherwise we would never exit the loop.
              *
+             * Same for CSC invalidation messages generated by signalModifiedKey.
+             *
              * AOF and Output buffer memory will be freed eventually so
              * we only care about memory used by the key space. */
             delta = (long long) zmalloc_used_memory();
@@ -570,12 +572,12 @@ int freeMemoryIfNeeded(void) {
                 dbAsyncDelete(db,keyobj);
             else
                 dbSyncDelete(db,keyobj);
-            signalModifiedKey(NULL,db,keyobj);
             latencyEndMonitor(eviction_latency);
             latencyAddSampleIfNeeded("eviction-del",eviction_latency);
             delta -= (long long) zmalloc_used_memory();
             mem_freed += delta;
             server.stat_evictedkeys++;
+            signalModifiedKey(NULL,db,keyobj);
             notifyKeyspaceEvent(NOTIFY_EVICTED, "evicted",
                 keyobj, db->id);
             decrRefCount(keyobj);

tests/support/redis.tcl

@@ -232,13 +232,20 @@ proc ::redis::redis_read_line fd {
     string trim [gets $fd]
 }
 
+proc ::redis::redis_read_null fd {
+    gets $fd
+    return {}
+}
+
 proc ::redis::redis_read_reply {id fd} {
     set type [read $fd 1]
     switch -exact -- $type {
+        _ {redis_read_null $fd}
         : -
         + {redis_read_line $fd}
         - {return -code error [redis_read_line $fd]}
         $ {redis_bulk_read $fd}
+        > -
         * {redis_multi_bulk_read $id $fd}
         % {redis_read_map $id $fd}
         default {

tests/unit/maxmemory.tcl

@@ -241,3 +241,53 @@ test_slave_buffers {slave buffer are counted correctly} 1000000 10 0 1
 # test again with fewer (and bigger) commands without pipeline, but with eviction
 test_slave_buffers "replica buffer don't induce eviction" 100000 100 1 0
 
+start_server {tags {"maxmemory"}} {
+    test {client tracking don't cause eviction feedback loop} {
+        r config set maxmemory 0
+        r config set maxmemory-policy allkeys-lru
+
+        # 10 clients listening on tracking messages
+        set clients {}
+        for {set j 0} {$j < 10} {incr j} {
+            lappend clients [redis_deferring_client]
+        }
+        foreach rd $clients {
+            $rd HELLO 3
+            $rd read ; # Consume the HELLO reply
+            $rd CLIENT TRACKING on
+            $rd read ; # Consume the CLIENT reply
+        }
+
+        # populate 300 keys, with long key name and short value
+        for {set j 0} {$j < 300} {incr j} {
+            set key $j[string repeat x 1000]
+            r set $key x
+
+            # for each key, enable caching for this key
+            foreach rd $clients {
+                $rd get $key
+                $rd read
+            }
+        }
+
+        # set the memory limit which will cause a few keys to be evicted
+        # we need to make sure to evict keynames of a total size of more than
+        # 16kb since the (PROTO_REPLY_CHUNK_BYTES), only after that the
+        # invalidation messages have a chance to trigger further eviction.
+        set used [s used_memory]
+        set limit [expr {$used - 40000}]
+        r config set maxmemory $limit
+
+        # make sure some eviction happened
+        set evicted [s evicted_keys]
+        if {$::verbose} { puts "evicted: $evicted" }
+        assert_range $evicted 10 50
+        foreach rd $clients {
+            $rd read ;# make sure we have some invalidation message waiting
+            $rd close
+        }
+
+        # make sure we didn't drain the database
+        assert_range [r dbsize] 200 300
+    }
+}