diff options
author | antirez <antirez@gmail.com> | 2019-10-10 10:23:34 +0200 |
---|---|---|
committer | antirez <antirez@gmail.com> | 2019-11-19 17:28:59 +0100 |
commit | 1a9e70c1d75c1bd7f29d659ef2b63a819bf5032a (patch) | |
tree | 231aaa21954c70abc1bdaed6cff3ade64a141a03 | |
parent | 69b1b5be6b67c65fede8c4120962d4edb887633b (diff) | |
download | redis-1a9e70c1d75c1bd7f29d659ef2b63a819bf5032a.tar.gz |
Cluster: fix memory leak of cached master.
This is what happened:
1. Instance starts, is a slave in the cluster configuration, but
actually server.masterhost is not set, so technically the instance
is acting like a master.
2. loadDataFromDisk() calls replicationCacheMasterUsingMyself() even if
the instance is a master, in the case it is logically a slave and the
cluster is enabled. So now we have a cached master even if the instance
is practically configured as a master (from the POV of
server.masterhost value and so forth).
3. clusterCron() sees that the instance requires to replicate from its
master, because logically it is a slave, so it calls
replicationSetMaster() that will in turn call
replicationCacheMasterUsingMyself(): before this commit, this call would
overwrite the old cached master, creating a memory leak.
-rw-r--r-- | src/replication.c | 5 | ||||
-rw-r--r-- | src/server.c | 8 |
2 files changed, 9 insertions, 4 deletions
diff --git a/src/replication.c b/src/replication.c index f8951ad14..a5e94db7a 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1996,7 +1996,10 @@ void replicationSetMaster(char *ip, int port) { cancelReplicationHandshake(); /* Before destroying our master state, create a cached master using * our own parameters, to later PSYNC with the new master. */ - if (was_master) replicationCacheMasterUsingMyself(); + if (was_master) { + replicationDiscardCachedMaster(); + replicationCacheMasterUsingMyself(); + } server.repl_state = REPL_STATE_CONNECT; } diff --git a/src/server.c b/src/server.c index f7fb4f882..6aeb5aa9a 100644 --- a/src/server.c +++ b/src/server.c @@ -3886,12 +3886,14 @@ void loadDataFromDisk(void) { (float)(ustime()-start)/1000000); /* Restore the replication ID / offset from the RDB file. */ - if ((server.masterhost || (server.cluster_enabled && nodeIsSlave(server.cluster->myself)))&& + if ((server.masterhost || + (server.cluster_enabled && + nodeIsSlave(server.cluster->myself))) && rsi.repl_id_is_set && rsi.repl_offset != -1 && /* Note that older implementations may save a repl_stream_db - * of -1 inside the RDB file in a wrong way, see more information - * in function rdbPopulateSaveInfo. */ + * of -1 inside the RDB file in a wrong way, see more + * information in function rdbPopulateSaveInfo. */ rsi.repl_stream_db != -1) { memcpy(server.replid,rsi.repl_id,sizeof(server.replid)); |