From 1a255e315024d4536cddba9d7ca6d06699d17be3 Mon Sep 17 00:00:00 2001
From: Eduardo Semprebon <eduardobr@gmail.com>
Date: Thu, 18 Nov 2021 09:53:17 +0100
Subject: Reject PING with MASTERDOWN when replica-serve-stale-data=no (#9757)

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.
---
 redis.conf           |  4 ++--
 src/server.c         | 13 +++++++------
 tests/unit/multi.tcl | 10 ++++++----
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/redis.conf b/redis.conf
index 04e8bb117..cccc48e00 100644
--- a/redis.conf
+++ b/redis.conf
@@ -520,8 +520,8 @@ dir ./
 #
 # 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'"
-#    to all data access commands, excluding commands such as :
-#    INFO, REPLICAOF, AUTH, PING, SHUTDOWN, REPLCONF, ROLE, CONFIG, SUBSCRIBE,
+#    to all data access commands, excluding commands such as:
+#    INFO, REPLICAOF, AUTH, SHUTDOWN, REPLCONF, ROLE, CONFIG, SUBSCRIBE,
 #    UNSUBSCRIBE, PSUBSCRIBE, PUNSUBSCRIBE, PUBLISH, PUBSUB, COMMAND, POST,
 #    HOST and LATENCY.
 #
diff --git a/src/server.c b/src/server.c
index a10fdd43c..09716f01e 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1516,11 +1516,12 @@ struct redisCommand redisCommandTable[] = {
     {"auth",authCommand,-2,
      "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
-     * failure detection, and a loading server is considered to be
-     * not available. */
+    /* PING is used for Redis failure detection and availability check.
+     * So we return LOADING in case there's a synchronous replication in progress,
+     * MASTERDOWN when replica-serve-stale-data=no and link with MASTER is down,
+     * BUSY when blocked by a script, etc. */
     {"ping",pingCommand,-1,
-     "ok-stale fast sentinel @connection"},
+     "fast sentinel @connection"},
 
     {"sentinel",NULL,-2,
      "admin only-sentinel",
@@ -5412,8 +5413,8 @@ int processCommand(client *c) {
         return C_OK;
     }
 
-    /* Only allow commands with flag "t", such as INFO, SLAVEOF and so on,
-     * when slave-serve-stale-data is no and we are a slave with a broken
+    /* Only allow commands with flag "t", such as INFO, REPLICAOF and so on,
+     * when replica-serve-stale-data is no and we are a replica with a broken
      * link with master. */
     if (server.masterhost && server.repl_state != REPL_STATE_CONNECTED &&
         server.repl_serve_stale_data == 0 &&
diff --git a/tests/unit/multi.tcl b/tests/unit/multi.tcl
index da15a7431..44757e1ab 100644
--- a/tests/unit/multi.tcl
+++ b/tests/unit/multi.tcl
@@ -500,19 +500,21 @@ start_server {tags {"multi"}} {
         set r1 [redis_client]
         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 get xx
         $r1 replicaof localhsot 0
         catch {r exec} e
         assert_match {*EXECABORT*MASTERDOWN*} $e
 
-        # check that PING is allowed
+        # reset
+        $r1 replicaof no one
+
         r multi
         r ping
         $r1 replicaof localhsot 0
-        set pong [r exec]
-        assert {$pong == "PONG"}
+        catch {r exec} e
+        assert_match {*EXECABORT*MASTERDOWN*} $e
 
         # 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
-- 
cgit v1.2.1