From 0e9b5adbd394837d711d8db34539a60cb181ba5b Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 5 Aug 2019 17:38:15 +0200 Subject: [PATCH] Replication: clarify why repl_put_online_on_ack exists at all. --- src/replication.c | 42 +++++++++++++++++++++++++++++++++--------- src/server.h | 2 +- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/replication.c b/src/replication.c index 26e7cf8f0825..d6646c9ef598 100644 --- a/src/replication.c +++ b/src/replication.c @@ -823,7 +823,9 @@ void replconfCommand(client *c) { c->repl_ack_time = server.unixtime; /* If this was a diskless replication, we need to really put * the slave online when the first ACK is received (which - * confirms slave is online and ready to get more data). */ + * confirms slave is online and ready to get more data). This + * allows for simpler and less CPU intensive EOF detection + * when streaming RDB files. */ if (c->repl_put_online_on_ack && c->replstate == SLAVE_STATE_ONLINE) putSlaveOnline(c); /* Note: this command does not reply anything! */ @@ -842,18 +844,20 @@ void replconfCommand(client *c) { addReply(c,shared.ok); } -/* This function puts a slave in the online state, and should be called just - * after a slave received the RDB file for the initial synchronization, and +/* This function puts a replica in the online state, and should be called just + * after a replica received the RDB file for the initial synchronization, and * we are finally ready to send the incremental stream of commands. * * It does a few things: * - * 1) Put the slave in ONLINE state (useless when the function is called - * because state is already ONLINE but repl_put_online_on_ack is true). + * 1) Put the slave in ONLINE state. Note that the function may also be called + * for a replicas that are already in ONLINE state, but having the flag + * repl_put_online_on_ack set to true: we still have to install the write + * handler in that case. This function will take care of that. * 2) Make sure the writable event is re-installed, since calling the SYNC * command disables it, so that we can accumulate output buffer without - * sending it to the slave. - * 3) Update the count of good slaves. */ + * sending it to the replica. + * 3) Update the count of "good replicas". */ void putSlaveOnline(client *slave) { slave->replstate = SLAVE_STATE_ONLINE; slave->repl_put_online_on_ack = 0; @@ -965,11 +969,31 @@ void updateSlavesWaitingBgsave(int bgsaveerr, int type) { serverLog(LL_NOTICE, "Streamed RDB transfer with replica %s succeeded (socket). Waiting for REPLCONF ACK from slave to enable streaming", replicationGetSlaveName(slave)); - /* Note: we wait for a REPLCONF ACK message from slave in + /* Note: we wait for a REPLCONF ACK message from the replica in * order to really put it online (install the write handler * so that the accumulated data can be transferred). However * we change the replication state ASAP, since our slave - * is technically online now. */ + * is technically online now. + * + * So things work like that: + * + * 1. We end trasnferring the RDB file via socket. + * 2. The replica is put ONLINE but the write handler + * is not installed. + * 3. The replica however goes really online, and pings us + * back via REPLCONF ACK commands. + * 4. Now we finally install the write handler, and send + * the buffers accumulated so far to the replica. + * + * But why we do that? Because the replica, when we stream + * the RDB directly via the socket, must detect the RDB + * EOF (end of file), that is a special random string at the + * end of the RDB (for streamed RDBs we don't know the length + * in advance). Detecting such final EOF string is much + * simpler and less CPU intensive if no more data is sent + * after such final EOF. So we don't want to glue the end of + * the RDB trasfer with the start of the other replication + * data. */ slave->replstate = SLAVE_STATE_ONLINE; slave->repl_put_online_on_ack = 1; slave->repl_ack_time = server.unixtime; /* Timeout otherwise. */ diff --git a/src/server.h b/src/server.h index 13e59683699c..d003ce59a906 100644 --- a/src/server.h +++ b/src/server.h @@ -848,7 +848,7 @@ typedef struct client { uint64_t flags; /* Client flags: CLIENT_* macros. */ int authenticated; /* Needed when the default user requires auth. */ int replstate; /* Replication state if this is a slave. */ - int repl_put_online_on_ack; /* Install slave write handler on ACK. */ + int repl_put_online_on_ack; /* Install slave write handler on first ACK. */ int repldbfd; /* Replication DB file descriptor. */ off_t repldboff; /* Replication DB file offset. */ off_t repldbsize; /* Replication DB file size. */