From 1a7cd2c0e25bd4488dd2c28f0ffea1b62dc0b3ad Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 15 May 2020 10:19:13 +0200
Subject: [PATCH] Cache master without checking of deferred close flags.

The context is issue #7205: since the introduction of threaded I/O we close
clients asynchronously by default from readQueryFromClient(). So we
should no longer prevent the caching of the master client, to later
PSYNC incrementally, if such flags are set. However we also don't want
the master client to be cached with such flags (would be closed
immediately after being restored). And yet we want a way to understand
if a master was closed because of a protocol error, and in that case
prevent the caching.
---
 src/networking.c  | 10 ++++------
 src/replication.c |  8 ++++++--
 src/server.h      |  1 +
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/networking.c b/src/networking.c
index 671e374f4..6de00be09 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -1141,10 +1141,7 @@ void freeClient(client *c) {
      * some unexpected state, by checking its flags. */
     if (server.master && c->flags & CLIENT_MASTER) {
         serverLog(LL_WARNING,"Connection with master lost.");
-        if (!(c->flags & (CLIENT_CLOSE_AFTER_REPLY|
-                          CLIENT_CLOSE_ASAP|
-                          CLIENT_BLOCKED)))
-        {
+        if (!(c->flags & (CLIENT_PROTOCOL_ERROR|CLIENT_BLOCKED))) {
             replicationCacheMaster(c);
             return;
         }
@@ -1558,7 +1555,8 @@ int processInlineBuffer(client *c) {
 }
 
 /* Helper function. Record protocol erro details in server log,
- * and set the client as CLIENT_CLOSE_AFTER_REPLY. */
+ * and set the client as CLIENT_CLOSE_AFTER_REPLY and
+ * CLIENT_PROTOCOL_ERROR. */
 #define PROTO_DUMP_LEN 128
 static void setProtocolError(const char *errstr, client *c) {
     if (server.verbosity <= LL_VERBOSE) {
@@ -1584,7 +1582,7 @@ static void setProtocolError(const char *errstr, client *c) {
             "Protocol error (%s) from client: %s. %s", errstr, client, buf);
         sdsfree(client);
     }
-    c->flags |= CLIENT_CLOSE_AFTER_REPLY;
+    c->flags |= (CLIENT_CLOSE_AFTER_REPLY|CLIENT_PROTOCOL_ERROR);
 }
 
 /* Process the query buffer for client 'c', setting up the client argument
diff --git a/src/replication.c b/src/replication.c
index c59639cd1..f433d6413 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -2688,14 +2688,18 @@ void replicationCacheMaster(client *c) {
     /* Unlink the client from the server structures. */
     unlinkClient(c);
 
+    /* Clear flags that can create issues once we reconnect the client. */
+    c->flags &= ~(CLIENT_CLOSE_ASAP|CLIENT_CLOSE_AFTER_REPLY);
+
     /* Reset the master client so that's ready to accept new commands:
      * we want to discard te non processed query buffers and non processed
      * offsets, including pending transactions, already populated arguments,
      * pending outputs to the master. */
     sdsclear(server.master->querybuf);
     sdsclear(server.master->pending_querybuf);
-    /* Adjust reploff and read_reploff to the last meaningful offset we executed.
-     * this is the offset the replica will use for future PSYNC. */
+
+    /* Adjust reploff and read_reploff to the last meaningful offset we
+     * executed. This is the offset the replica will use for future PSYNC. */
     server.master->reploff = adjustMeaningfulReplOffset();
     server.master->read_reploff = server.master->reploff;
     if (c->flags & CLIENT_MULTI) discardTransaction(c);
diff --git a/src/server.h b/src/server.h
index 55ee2d300..f835bf5e9 100644
--- a/src/server.h
+++ b/src/server.h
@@ -255,6 +255,7 @@ typedef long long ustime_t; /* microsecond time type. */
 #define CLIENT_TRACKING_NOLOOP (1ULL<<37) /* Don't send invalidation messages
                                              about writes performed by myself.*/
 #define CLIENT_IN_TO_TABLE (1ULL<<38) /* This client is in the timeout table. */
+#define CLIENT_PROTOCOL_ERROR (1ULL<<39) /* Protocol error chatting with it. */
 
 /* Client block type (btype field in client structure)
  * if CLIENT_BLOCKED flag is set. */