Bug#981226: redis: New systemd integration breaks replicaof handling

[Guillem Jover]

Source: redis
Source-Version: 5:6.0.9-3
Severity: important
Tags: upstream patch
Forwarded: https://github.com/redis/redis/pull/8409

[ This might be worth being serious, but I'm not sure this falls under
  uncommon usage or similar, or not, so please bump if you agree. :) ]

Hi!

While preparing to switch to bullseye, we noticed that the new systemd
integration breaks with replicaof handling.

I've submitted this upstream, but it would be nice to fix in Debian
even if upstream does not apply this in time for the freeze.

The github PR and the attached patch against the version in Debian,
describe the problem in further detail.

Thanks,
Guillem

From f9505104123664c45b762b8a7bb15406c205c01d Mon Sep 17 00:00:00 2001
From: Guillem Jover <gjo...@sipwise.com>
Date: Tue, 19 Jan 2021 00:34:59 +0100
Subject: [PATCH] Send the readiness notification when we are ready to accept
 connections

On a replica we do accept connections, even though commands accessing
the database will operate in read-only mode. But the server is still
already operational and processing commands.

Not sending the readiness notification means that on a HA setup where
the nodes all start as replicas (with replicaof in the config) with
a replica that cannot connect to the master server and which might not
come back in a predictable amount of time or at all, the service
supervisor will end up timing out the service and terminating it, with
no option to promote it to be the main instance. This seems counter to
what the readiness notification is supposed to be signaling.

Instead send the readiness notification when we start accepting
commands, and then send the various server status changes as that.

Fixes: commit 641c64ada10404356fc76c0b56a69b32c76f253c
Fixes: commit dfb598cf3304818e608ceb6b5d9529a293345c4a
---
 src/replication.c |    6 ++----
 src/server.c      |    4 ++--
 2 files changed, 4 insertions(+), 6 deletions(-)

--- a/src/replication.c
+++ b/src/replication.c
@@ -1832,8 +1832,7 @@ void readSyncBulkPayload(connection *con
     serverLog(LL_NOTICE, "MASTER <-> REPLICA sync: Finished with success");
 
     if (server.supervised_mode == SUPERVISED_SYSTEMD) {
-        redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Finished with success. Ready to accept connections.\n");
-        redisCommunicateSystemd("READY=1\n");
+        redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Finished with success. Ready to accept connections in read-write mode.\n");
     }
 
     /* Restart the AOF subsystem now that we finished the sync. This
@@ -2340,8 +2339,7 @@ void syncWithMaster(connection *conn) {
     if (psync_result == PSYNC_CONTINUE) {
         serverLog(LL_NOTICE, "MASTER <-> REPLICA sync: Master accepted a Partial Resynchronization.");
         if (server.supervised_mode == SUPERVISED_SYSTEMD) {
-            redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Partial Resynchronization accepted. Ready to accept connections.\n");
-            redisCommunicateSystemd("READY=1\n");
+            redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Partial Resynchronization accepted. Ready to accept connections in read-write mode.\n");
         }
         return;
     }
--- a/src/server.c
+++ b/src/server.c
@@ -5324,10 +5324,10 @@ int main(int argc, char **argv) {
         if (server.supervised_mode == SUPERVISED_SYSTEMD) {
             if (!server.masterhost) {
                 redisCommunicateSystemd("STATUS=Ready to accept connections\n");
-                redisCommunicateSystemd("READY=1\n");
             } else {
-                redisCommunicateSystemd("STATUS=Waiting for MASTER <-> REPLICA sync\n");
+                redisCommunicateSystemd("STATUS=Ready to accept connections in read-only mode. Waiting for MASTER <-> REPLICA sync\n");
             }
+            redisCommunicateSystemd("READY=1\n");
         }
     } else {
         InitServerLast();