zhuyijun/read-redis
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

When redis-cli received ASK, it didn't handle it (#8930)

Browse Source 
When redis-cli received ASK, it used string matching wrong and didn't
handle it.

When we access a slot which is in migrating state, it maybe
return ASK. After redirect to the new node, we need send ASKING
command before retry the command.  In this PR after redis-cli receives
ASK, we send a ASKING command before send the origin command
after reconnecting.

Other changes:
* Make redis-cli -u and -c (unix socket and cluster mode) incompatible
  with one another.
* When send command fails, we avoid the 2nd reconnect retry and just
  print the error info. Users will decide how to do next.
  See #9277.
* Add a test faking two redis nodes in TCL to just send ASK and OK in
  redis protocol to test ASK behavior.

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit cf61ad14cc45787e57d9af3f28f41462ac0f2aa2)
This commit is contained in:
Huang Zhw 2021-08-02 19:59:08 +08:00 committed by Oran Agra
parent 9da232c05a
commit 8892b5cf9e
3 changed files with 135 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
src/redis-cli.c
View File

@ -220,6 +220,7 @@ static struct config {
long long lru_test_sample_size;
int cluster_mode;
int cluster_reissue_command;
int cluster_send_asking;
int slave_mode;
int pipe_mode;
int pipe_timeout;
@ -924,6 +925,29 @@ static int cliConnect(int flags) {
return REDIS_OK;
}
/* In cluster, if server replies ASK, we will redirect to a different node.
* Before sending the real command, we need to send ASKING command first. */
static int cliSendAsking() {
redisReply *reply;
config.cluster_send_asking = 0;
if (context == NULL) {
return REDIS_ERR;
}
reply = redisCommand(context,"ASKING");
if (reply == NULL) {
fprintf(stderr, "\nI/O error\n");
return REDIS_ERR;
}
int result = REDIS_OK;
if (reply->type == REDIS_REPLY_ERROR) {
result = REDIS_ERR;
fprintf(stderr,"ASKING failed: %s\n",reply->str);
}
freeReplyObject(reply);
return result;
}
static void cliPrintContextError(void) {
if (context == NULL) return;
fprintf(stderr,"Error: %s\n",context->errstr);
@ -1299,7 +1323,7 @@ static int cliReadReply(int output_raw_strings) {
/* Check if we need to connect to a different node and reissue the
* request. */
if (config.cluster_mode && reply->type == REDIS_REPLY_ERROR &&
(!strncmp(reply->str,"MOVED",5) || !strcmp(reply->str,"ASK")))
(!strncmp(reply->str,"MOVED ",6) || !strncmp(reply->str,"ASK ",4)))
{
char *p = reply->str, *s;
int slot;
@ -1323,6 +1347,9 @@ static int cliReadReply(int output_raw_strings) {
printf("-> Redirected to slot [%d] located at %s:%d\n",
slot, config.hostip, config.hostport);
config.cluster_reissue_command = 1;
if (!strncmp(reply->str,"ASK ",4)) {
config.cluster_send_asking = 1;
}
cliRefreshPrompt();
} else if (!config.interactive && config.set_errcode &&
reply->type == REDIS_REPLY_ERROR)
@ -1804,6 +1831,11 @@ static int parseOptions(int argc, char **argv) {
}
}
if (config.hostsocket && config.cluster_mode) {
fprintf(stderr,"Options -c and -s are mutually exclusive.\n");
exit(1);
}
/* --ldb requires --eval. */
if (config.eval_ldb && config.eval == NULL) {
fprintf(stderr,"Options --ldb and --ldb-sync-mode require --eval.\n");
@ -2021,26 +2053,32 @@ static sds *getSdsArrayFromArgv(int argc, char **argv, int quoted) {
static int issueCommandRepeat(int argc, char **argv, long repeat) {
while (1) {
if (config.cluster_reissue_command || context == NULL ||
context->err == REDIS_ERR_IO || context->err == REDIS_ERR_EOF)
{
if (cliConnect(CC_FORCE) != REDIS_OK) {
cliPrintContextError();
config.cluster_reissue_command = 0;
return REDIS_ERR;
}
}
config.cluster_reissue_command = 0;
if (cliSendCommand(argc,argv,repeat) != REDIS_OK) {
cliConnect(CC_FORCE);
/* If we still cannot send the command print error.
* We'll try to reconnect the next time. */
if (cliSendCommand(argc,argv,repeat) != REDIS_OK) {
if (config.cluster_send_asking) {
if (cliSendAsking() != REDIS_OK) {
cliPrintContextError();
return REDIS_ERR;
}
}
if (cliSendCommand(argc,argv,repeat) != REDIS_OK) {
cliPrintContextError();
return REDIS_ERR;
}
/* Issue the command again if we got redirected in cluster mode */
if (config.cluster_mode && config.cluster_reissue_command) {
/* If cliConnect fails, sleep for a while and try again. */
if (cliConnect(CC_FORCE) != REDIS_OK)
sleep(1);
} else {
break;
continue;
}
break;
}
return REDIS_OK;
}
@ -8248,6 +8286,7 @@ int main(int argc, char **argv) {
config.lru_test_mode = 0;
config.lru_test_sample_size = 0;
config.cluster_mode = 0;
config.cluster_send_asking = 0;
config.slave_mode = 0;
config.getrdb_mode = 0;
config.stat_mode = 0;

58
tests/helpers/fake_redis_node.tcl Normal file
View File

@ -0,0 +1,58 @@
# A fake Redis node for replaying predefined/expected traffic with a client.
#
# Usage: tclsh fake_redis_node.tcl PORT COMMAND REPLY [ COMMAND REPLY [ ... ] ]
#
# Commands are given as space-separated strings, e.g. "GET foo", and replies as
# RESP-encoded replies minus the trailing \r\n, e.g. "+OK".
set port [lindex $argv 0];
set expected_traffic [lrange $argv 1 end];
# Reads and parses a command from a socket and returns it as a space-separated
# string, e.g. "set foo bar".
proc read_command {sock} {
set char [read $sock 1]
switch $char {
* {
set numargs [gets $sock]
set result {}
for {set i 0} {$i<$numargs} {incr i} {
read $sock 1; # dollar sign
set len [gets $sock]
set str [read $sock $len]
gets $sock; # trailing \r\n
lappend result $str
}
return $result
}
{} {
# EOF
return {}
}
default {
# Non-RESP command
set rest [gets $sock]
return "$char$rest"
}
}
}
proc accept {sock host port} {
global expected_traffic
foreach {expect_cmd reply} $expected_traffic {
if {[eof $sock]} {break}
set cmd [read_command $sock]
if {[string equal -nocase $cmd $expect_cmd]} {
puts $sock $reply
flush $sock
} else {
puts $sock "-ERR unexpected command $cmd"
break
}
}
close $sock
}
socket -server accept $port
after 5000 set done timeout
vwait done

31
tests/integration/redis-cli.tcl
View File

@ -64,8 +64,8 @@ start_server {tags {"cli"}} {
set _ $tmp
}
proc _run_cli {opts args} {
set cmd [rediscli [srv host] [srv port] [list -n 9 {*}$args]]
proc _run_cli {host port db opts args} {
set cmd [rediscli $host $port [list -n $db {*}$args]]
foreach {key value} $opts {
if {$key eq "pipe"} {
set cmd "sh -c \"$value | $cmd\""
@ -84,15 +84,19 @@ start_server {tags {"cli"}} {
}
proc run_cli {args} {
_run_cli {} {*}$args
_run_cli [srv host] [srv port] 9 {} {*}$args
}
proc run_cli_with_input_pipe {cmd args} {
_run_cli [list pipe $cmd] -x {*}$args
_run_cli [srv host] [srv port] 9 [list pipe $cmd] -x {*}$args
}
proc run_cli_with_input_file {path args} {
_run_cli [list path $path] -x {*}$args
_run_cli [srv host] [srv port] 9 [list path $path] -x {*}$args
}
proc run_cli_host_port_db {host port db args} {
_run_cli $host $port $db {} {*}$args
}
proc test_nontty_cli {name code} {
@ -207,6 +211,23 @@ start_server {tags {"cli"}} {
assert_equal "foo\nbar" [run_cli lrange list 0 -1]
}
test_nontty_cli "ASK redirect test" {
# Set up two fake Redis nodes.
set tclsh [info nameofexecutable]
set script "tests/helpers/fake_redis_node.tcl"
set port1 [find_available_port $::baseport $::portcount]
set port2 [find_available_port $::baseport $::portcount]
set p1 [exec $tclsh $script $port1 \
"SET foo bar" "-ASK 12182 127.0.0.1:$port2" &]
set p2 [exec $tclsh $script $port2 \
"ASKING" "+OK" \
"SET foo bar" "+OK" &]
# Sleep to make sure both fake nodes have started listening
after 100
# Run the cli
assert_equal "OK" [run_cli_host_port_db "127.0.0.1" $port1 0 -c SET foo bar]
}
test_nontty_cli "Quoted input arguments" {
r set "\x00\x00" "value"
assert_equal "value" [run_cli --quoted-input get {"\x00\x00"}]