New config: lazyexpire-nested-arbitrary-keys (#14149)

In this PR we added hidden config - `lazyexpire-nested-arbitrary-keys`,
which can take:
* yes - the default. produce and propagate lazy-expire DELs as usual.
* no - avoid lazy-expire from commands that touch arbitrary keys (such
as SCAN, RANDOMKEY), if generated within a transactions (MULTI/EXEC,
LUA). This ensures such commands won't induce CROSSSLOT on remote proxy,
as happened in when replicating one db into another (possibly sharded
differently).
Since the issue is relevant only in replicated servers (RE's replica-of
mode or CRDT) - it was added to the core as a hidden config.

Please note that this config will always apply to read-only commands
(see EXPIRE_FORCE_DELETE_EXPIRED flag).
Since write commands may require key expiration to operate correctly.

---------

Co-authored-by: debing.sun <debing.sun@redis.com>

src/config.c

@@ -3114,6 +3114,7 @@ standardConfig static_configs[] = {
     createBoolConfig("aof-disable-auto-gc", NULL, MODIFIABLE_CONFIG | HIDDEN_CONFIG, server.aof_disable_auto_gc, 0, NULL, updateAofAutoGCEnabled),
     createBoolConfig("replica-ignore-disk-write-errors", NULL, MODIFIABLE_CONFIG, server.repl_ignore_disk_write_error, 0, NULL, NULL),
     createBoolConfig("hide-user-data-from-log", NULL, MODIFIABLE_CONFIG, server.hide_user_data_from_log, 0, NULL, NULL),
+    createBoolConfig("lazyexpire-nested-arbitrary-keys", NULL, MODIFIABLE_CONFIG | HIDDEN_CONFIG, server.lazyexpire_nested_arbitrary_keys, 1, NULL, NULL),
 
     /* String Configs */
     createStringConfig("aclfile", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.acl_filename, "", NULL, NULL),

src/db.c

@@ -2444,6 +2444,17 @@ int keyIsExpired(redisDb *db, sds key, kvobj *kv) {
     return now > when;
 }
 
+/* Check if user configuration allows key to be deleted due to expiary */
+int confAllowsExpireDel(void) {
+    if (server.lazyexpire_nested_arbitrary_keys)
+        return 1;
+
+    /* This configuration specifically targets nested commands, to align with RE's feature of replication between dbs.
+     * transactions (from scripts or multi-exec) containing commands like SCAN and RANDOMKEY will execute locally, but their
+     * lazy-expiration DELs may induce CROSS-SLOT on remote proxy in mode replica-of (RED-161574) */
+    return !(server.execution_nesting > 1 && server.executing_client->cmd->flags & CMD_TOUCHES_ARBITRARY_KEYS);
+}
+
 /* This function is called when we are going to perform some operation
  * in a given key, but such key may be already logically expired even if
  * it still exists in the database. The main way this function is called
@@ -2503,6 +2514,11 @@ keyStatus expireIfNeeded(redisDb *db, robj *key, kvobj *kv, int flags) {
         if (!(flags & EXPIRE_FORCE_DELETE_EXPIRED)) return KEY_EXPIRED;
     }
 
+    /* Check if user configuration disables lazy-expire deletions in current state.
+     * This will only apply if the server doesn't mandate key deletion to operate correctly (write commands). */
+    if (!(flags & EXPIRE_FORCE_DELETE_EXPIRED) && !confAllowsExpireDel())
+        return KEY_EXPIRED;
+
     /* In some cases we're explicitly instructed to return an indication of a
      * missing key without actually deleting it, even on masters. */
     if (flags & EXPIRE_AVOID_DELETE_EXPIRED)

src/server.h

@@ -1977,6 +1977,7 @@ struct redisServer {
     unsigned int max_new_tls_conns_per_cycle; /* The maximum number of tls connections that will be accepted during each invocation of the event loop. */
     unsigned int max_new_conns_per_cycle; /* The maximum number of tcp connections that will be accepted during each invocation of the event loop. */
     int cluster_compatibility_sample_ratio; /* Sampling ratio for cluster mode incompatible commands. */
+    int lazyexpire_nested_arbitrary_keys; /* If disabled, avoid lazy-expire from commands that touch arbitrary keys (SCAN/RANDOMKEY) within transactions */
 
     /* AOF persistence */
     int aof_enabled;                /* AOF configuration */
@@ -3620,6 +3621,7 @@ void deleteExpiredKeyAndPropagate(redisDb *db, robj *keyobj);
 void deleteEvictedKeyAndPropagate(redisDb *db, robj *keyobj, long long *key_mem_freed);
 void propagateDeletion(redisDb *db, robj *key, int lazy);
 int keyIsExpired(redisDb *db, sds key, kvobj *kv);
+int confAllowsExpireDel(void);
 long long getExpire(redisDb *db, sds key, kvobj *kv);
 kvobj *setExpire(client *c, redisDb *db, robj *key, long long when);
 kvobj *setExpireByLink(client *c, redisDb *db, sds key, long long when, dictEntryLink link);

tests/unit/expire.tcl

@@ -899,3 +899,59 @@ start_cluster 1 0 {tags {"expire external:skip cluster"}} {
         assert_equal 0 [s 0 expired_time_cap_reached_count]
     } {} {needs:debug}
 }
+
+# Config lazyexpire-nested-arbitrary-keys test body
+proc conf_le_test {option mode} {
+    r config set lazyexpire-nested-arbitrary-keys $option
+    r debug set-active-expire 0
+    r flushall
+    r script LOAD {return redis.call('SCAN', 0)}
+
+    r set foo bar
+    r pexpire foo 1
+    after 2
+
+    set repl [attach_to_replication_stream]
+
+    # First two conditions hit lazy expire within a 'transaction', meaning
+    # DEL propagation should be blocked if 'lazyexpire-nested-arbitrary-keys' is set.
+    if {$mode == "lua"} {
+        r eval "return redis.call('SCAN', 0)" 0
+    } elseif {$mode == "multi"} {
+        r multi
+        r scan 0
+        r exec
+    } else {
+        r scan 0
+    }
+
+    # dummy command to verify nothing else gets into the replication stream.
+    r set x 1
+
+    if {$option == "no" && $mode != "direct"} {
+        assert_replication_stream $repl {
+            {select *}
+            {set x 1}
+        }
+    } else {
+        assert_replication_stream $repl {
+            {select *}
+            {del foo}
+            {set x 1}
+        }
+    }
+
+    close_replication_stream $repl
+    r script flush
+    assert_equal [r config set lazyexpire-nested-arbitrary-keys yes] {OK}
+    assert_equal [r debug set-active-expire 1] {OK}
+}
+
+foreach option {yes no} {
+foreach mode {direct multi lua} {
+    start_server {tags {"expire"}} {
+        test "Config lazyexpire-nested-arbitrary-keys ($option, $mode)" {
+            conf_le_test $option $mode
+        } {} {needs:debug repl}
+    }
+}}