From f6029fb9256ee1ffa1491f6fa57bb56a9558e952 Mon Sep 17 00:00:00 2001
From: WuYunlong <xzsyeb@126.com>
Date: Wed, 18 Mar 2020 16:20:10 +0800
Subject: [PATCH] Fix master replica inconsistency for upgrading scenario.

Before this commit, when upgrading a replica, expired keys will not
be loaded, thus causing replica having less keys in db. To this point,
master and replica's keys is logically consistent. However, before
the keys in master and replica are physically consistent, that is,
they have the same dbsize, if master got a problem and the replica
got promoted and becomes new master of that partition, and master
updates a key which does not exist on master, but physically exists
on the old master(new replica), the old master would refuse to update
the key, thus causing master and replica data inconsistent.

How could this happen?
That's all because of the wrong judgement of roles while starting up
the server. We can not use server.masterhost to judge if the server
is master or replica, since it fails in cluster mode.

When we start the server, we load rdb and do want to load expired keys,
and do not want to have the ability to active expire keys, if it is
a replica.
---
 src/rdb.c    | 2 +-
 src/server.c | 7 ++++++-
 src/server.h | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/rdb.c b/src/rdb.c
index cbcea96c6..5d34f5a32 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -2231,7 +2231,7 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) {
          * received from the master. In the latter case, the master is
          * responsible for key expiry. If we would expire keys here, the
          * snapshot taken by the master may not be reflected on the slave. */
-        if (server.masterhost == NULL && !(rdbflags&RDBFLAGS_AOF_PREAMBLE) && expiretime != -1 && expiretime < now) {
+        if (iAmMaster() && !(rdbflags&RDBFLAGS_AOF_PREAMBLE) && expiretime != -1 && expiretime < now) {
             decrRefCount(key);
             decrRefCount(val);
         } else {
diff --git a/src/server.c b/src/server.c
index f702da94a..467d09b67 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1691,7 +1691,7 @@ void databasesCron(void) {
     /* Expire keys by random sampling. Not required for slaves
      * as master will synthesize DELs for us. */
     if (server.active_expire_enabled) {
-        if (server.masterhost == NULL) {
+        if (iAmMaster()) {
             activeExpireCycle(ACTIVE_EXPIRE_CYCLE_SLOW);
         } else {
             expireSlaveKeys();
@@ -4863,6 +4863,11 @@ int redisIsSupervised(int mode) {
     return 0;
 }
 
+int iAmMaster(void) {
+    return ((!server.cluster_enabled && server.masterhost == NULL) ||
+            (server.cluster_enabled && nodeIsMaster(server.cluster->myself)));
+}
+
 
 int main(int argc, char **argv) {
     struct timeval tv;
diff --git a/src/server.h b/src/server.h
index fa6770dfa..fdfe5b8ea 100644
--- a/src/server.h
+++ b/src/server.h
@@ -2393,4 +2393,6 @@ int tlsConfigure(redisTLSContextConfig *ctx_config);
 #define redisDebugMark() \
     printf("-- MARK %s:%d --\n", __FILE__, __LINE__)
 
+int iAmMaster(void);
+
 #endif