slipenkomk/valkey
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Reject PING with MASTERDOWN when replica-serve-stale-data=no (#9757)

Browse Source 
Currently PING returns different status when server is not serving data,
for example when `LOADING` or `BUSY`.
But same was not true for `MASTERDOWN`
This commit makes PING reply with `MASTERDOWN` when
replica-serve-stale-data=no and link is MASTER is down.
This commit is contained in:
Eduardo Semprebon 2021-11-18 09:53:17 +01:00 committed by GitHub
parent af7489886d
commit 1a255e3150
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 15 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
redis.conf
View File

@ -520,8 +520,8 @@ dir ./
# #
# 2) If replica-serve-stale-data is set to 'no' the replica will reply with error # 2) If replica-serve-stale-data is set to 'no' the replica will reply with error
# "MASTERDOWN Link with MASTER is down and replica-serve-stale-data is set to 'no'" # "MASTERDOWN Link with MASTER is down and replica-serve-stale-data is set to 'no'"
# to all data access commands, excluding commands such as : # to all data access commands, excluding commands such as:
# INFO, REPLICAOF, AUTH, PING, SHUTDOWN, REPLCONF, ROLE, CONFIG, SUBSCRIBE, # INFO, REPLICAOF, AUTH, SHUTDOWN, REPLCONF, ROLE, CONFIG, SUBSCRIBE,
# UNSUBSCRIBE, PSUBSCRIBE, PUNSUBSCRIBE, PUBLISH, PUBSUB, COMMAND, POST, # UNSUBSCRIBE, PSUBSCRIBE, PUNSUBSCRIBE, PUBLISH, PUBSUB, COMMAND, POST,
# HOST and LATENCY. # HOST and LATENCY.
# #

13
src/server.c
View File

@ -1516,11 +1516,12 @@ struct redisCommand redisCommandTable[] = {
{"auth",authCommand,-2, {"auth",authCommand,-2,
"no-auth no-script ok-loading ok-stale fast sentinel @connection"}, "no-auth no-script ok-loading ok-stale fast sentinel @connection"},
/* We don't allow PING during loading since in Redis PING is used as /* PING is used for Redis failure detection and availability check.
* failure detection, and a loading server is considered to be * So we return LOADING in case there's a synchronous replication in progress,
* not available. */ * MASTERDOWN when replica-serve-stale-data=no and link with MASTER is down,
* BUSY when blocked by a script, etc. */
{"ping",pingCommand,-1, {"ping",pingCommand,-1,
"ok-stale fast sentinel @connection"}, "fast sentinel @connection"},
{"sentinel",NULL,-2, {"sentinel",NULL,-2,
"admin only-sentinel", "admin only-sentinel",
@ -5412,8 +5413,8 @@ int processCommand(client *c) {
return C_OK; return C_OK;
} }
/* Only allow commands with flag "t", such as INFO, SLAVEOF and so on, /* Only allow commands with flag "t", such as INFO, REPLICAOF and so on,
* when slave-serve-stale-data is no and we are a slave with a broken * when replica-serve-stale-data is no and we are a replica with a broken
* link with master. */ * link with master. */
if (server.masterhost && server.repl_state != REPL_STATE_CONNECTED && if (server.masterhost && server.repl_state != REPL_STATE_CONNECTED &&
server.repl_serve_stale_data == 0 && server.repl_serve_stale_data == 0 &&

10
tests/unit/multi.tcl
View File

@ -500,19 +500,21 @@ start_server {tags {"multi"}} {
set r1 [redis_client] set r1 [redis_client]
r set xx 1 r set xx 1
# check that GET is disallowed on stale replica, even if the replica becomes stale only after queuing. # check that GET and PING are disallowed on stale replica, even if the replica becomes stale only after queuing.
r multi r multi
r get xx r get xx
$r1 replicaof localhsot 0 $r1 replicaof localhsot 0
catch {r exec} e catch {r exec} e
assert_match {*EXECABORT*MASTERDOWN*} $e assert_match {*EXECABORT*MASTERDOWN*} $e
# check that PING is allowed # reset
$r1 replicaof no one
r multi r multi
r ping r ping
$r1 replicaof localhsot 0 $r1 replicaof localhsot 0
set pong [r exec] catch {r exec} e
assert {$pong == "PONG"} assert_match {*EXECABORT*MASTERDOWN*} $e
# check that when replica is not stale, GET is allowed # check that when replica is not stale, GET is allowed
# while we're at it, let's check that multi is allowed on stale replica too # while we're at it, let's check that multi is allowed on stale replica too