From 743f5ac2ae18080658a862e483d8247c3b2967df Mon Sep 17 00:00:00 2001
From: "zhaozhao.zz" <zhaozhao.zz@alibaba-inc.com>
Date: Fri, 30 Aug 2024 10:17:53 +0800
Subject: [PATCH] standalone -REDIRECT handles special case of MULTI context
 (#895)

In standalone mode, when a `-REDIRECT` error occurs, special handling is
required if the client is in the `MULTI` context.

We have adopted the same handling method as the cluster mode:

1. If a command in the transaction encounters a `REDIRECT` at the time
of queuing, the execution of `EXEC` will return an `EXECABORT` error (we
expect the client to redirect and discard the transaction upon receiving
a `REDIRECT`). That is:

    ```
    MULTI    ==>  +OK
    SET x y  ==>  -REDIRECT
    EXEC     ==>  -EXECABORT
    ```
2. If all commands are successfully queued (i.e., `QUEUED` results are
received) but a redirect is detected during `EXEC` execution (such as a
primary-replica switch), a `REDIRECT` is returned to instruct the client
to perform a redirect. That is:

    ```
    MULTI    ==>  +OK
    SET x y  ==>  +QUEUED
    failover
    EXEC     ==>  -REDIRECT
    ```

---------

Signed-off-by: zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
---
 src/server.c                                  |  7 ++++++
 tests/integration/replica-redirect.tcl        | 24 +++++++++++++++++--
 .../unit/cluster/transactions-on-replica.tcl  | 24 +++++++++++++++++++
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/src/server.c b/src/server.c
index f343ff1bba..3aaddd38d8 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3943,6 +3943,13 @@ int processCommand(client *c) {
              * and then resume the execution. */
             blockPostponeClient(c);
         } else {
+            if (c->cmd->proc == execCommand) {
+                discardTransaction(c);
+            } else {
+                flagTransaction(c);
+            }
+            c->duration = 0;
+            c->cmd->rejected_calls++;
             addReplyErrorSds(c, sdscatprintf(sdsempty(), "-REDIRECT %s:%d", server.primary_host, server.primary_port));
         }
         return C_OK;
diff --git a/tests/integration/replica-redirect.tcl b/tests/integration/replica-redirect.tcl
index b62ce4ca3b..b4e5a74b66 100644
--- a/tests/integration/replica-redirect.tcl
+++ b/tests/integration/replica-redirect.tcl
@@ -9,8 +9,28 @@ start_server {tags {needs:repl external:skip}} {
         set replica_port [srv 0 port]
         set replica_pid [srv 0 pid]
 
-        r replicaof $primary_host $primary_port
-        wait_replica_online $primary
+        test {write command inside MULTI is QUEUED, EXEC should be REDIRECT} {
+            set rr [valkey_client]
+            $rr client capa redirect
+            $rr multi
+            assert_equal "QUEUED" [$rr set foo bar]
+
+            # Change the current instance to be a replica
+            r replicaof $primary_host $primary_port
+            wait_replica_online $primary
+
+            assert_error "REDIRECT*" {$rr exec}
+            $rr close
+        }
+
+        test {write command inside MULTI is REDIRECT, EXEC should be EXECABORT} {
+            set rr [valkey_client]
+            $rr client capa redirect
+            $rr multi
+            assert_error "REDIRECT*" {$rr set foo bar}
+            assert_error "EXECABORT*" {$rr exec}
+            $rr close
+        }
 
         test {replica allow read command by default} {
             r get foo
diff --git a/tests/unit/cluster/transactions-on-replica.tcl b/tests/unit/cluster/transactions-on-replica.tcl
index b53af58cac..5bca9e6a37 100644
--- a/tests/unit/cluster/transactions-on-replica.tcl
+++ b/tests/unit/cluster/transactions-on-replica.tcl
@@ -53,6 +53,30 @@ test "MULTI-EXEC with write operations is MOVED" {
     assert {[string range $err 0 8] eq {EXECABORT}}
 }
 
+test "write command is QUEUED, then EXEC should be MOVED after failover" {
+    set rr [valkey_client]
+    $rr MULTI
+    assert {[$rr SET foo bar] eq {QUEUED}}
+
+    $replica CLUSTER FAILOVER FORCE
+    wait_for_condition 50 1000 {
+        [status $primary master_link_status] == "up"
+    } else {
+        fail "FAILOVER failed."
+    }
+
+    catch {$rr EXEC} err
+    assert {[string range $err 0 4] eq {MOVED}} 
+    $rr close
+
+    $primary CLUSTER FAILOVER FORCE
+    wait_for_condition 50 1000 {
+        [status $replica master_link_status] == "up"
+    } else {
+        fail "FAILOVER failed."
+    }
+}
+
 test "read-only blocking operations from replica" {
     set rd [valkey_deferring_client -1]
     $rd readonly