Skip to content

Commit

Permalink
standalone -REDIRECT handles special case of MULTI context (#895)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
soloestoy authored Aug 30, 2024
1 parent 2b76c8f commit 743f5ac
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 2 deletions.
7 changes: 7 additions & 0 deletions src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
24 changes: 22 additions & 2 deletions tests/integration/replica-redirect.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions tests/unit/cluster/transactions-on-replica.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 743f5ac

Please sign in to comment.