diff --git a/src/networking.c b/src/networking.c index 3b4f3b0f..761645cb 100644 --- a/src/networking.c +++ b/src/networking.c @@ -135,23 +135,49 @@ redisClient *createClient(int fd) { * returns REDIS_OK, and make sure to install the write handler in our event * loop so that when the socket is writable new data gets written. * - * If the client should not receive new data, because it is a fake client, - * a master, a slave not yet online, or because the setup of the write handler - * failed, the function returns REDIS_ERR. + * If the client should not receive new data, because it is a fake client + * (used to load AOF in memory), a master or because the setup of the write + * handler failed, the function returns REDIS_ERR. + * + * The function may return REDIS_OK without actually installing the write + * event handler in the following cases: + * + * 1) The event handler should already be installed since the output buffer + * already contained something. + * 2) The client is a slave but not yet online, so we want to just accumulate + * writes in the buffer but not actually sending them yet. * * Typically gets called every time a reply is built, before adding more * data to the clients output buffers. If the function returns REDIS_ERR no * data should be appended to the output buffers. */ int prepareClientToWrite(redisClient *c) { + /* If it's the Lua client we always return ok without installing any + * handler since there is no socket at all. */ if (c->flags & REDIS_LUA_CLIENT) return REDIS_OK; + + /* Masters don't receive replies, unless REDIS_MASTER_FORCE_REPLY flag + * is set. */ if ((c->flags & REDIS_MASTER) && !(c->flags & REDIS_MASTER_FORCE_REPLY)) return REDIS_ERR; - if (c->fd <= 0) return REDIS_ERR; /* Fake client */ + + if (c->fd <= 0) return REDIS_ERR; /* Fake client for AOF loading. */ + + /* Only install the handler if not already installed and, in case of + * slaves, if the client can actually receive writes. */ if (c->bufpos == 0 && listLength(c->reply) == 0 && (c->replstate == REDIS_REPL_NONE || - c->replstate == REDIS_REPL_ONLINE) && !c->repl_put_online_on_ack && - aeCreateFileEvent(server.el, c->fd, AE_WRITABLE, - sendReplyToClient, c) == AE_ERR) return REDIS_ERR; + (c->replstate == REDIS_REPL_ONLINE && !c->repl_put_online_on_ack))) + { + /* Try to install the write handler. */ + if (aeCreateFileEvent(server.el, c->fd, AE_WRITABLE, + sendReplyToClient, c) == AE_ERR) + { + freeClientAsync(c); + return REDIS_ERR; + } + } + + /* Authorize the caller to queue in the output buffer of this client. */ return REDIS_OK; } diff --git a/src/replication.c b/src/replication.c index c01cd52e..ca263527 100644 --- a/src/replication.c +++ b/src/replication.c @@ -652,7 +652,8 @@ void replconfCommand(redisClient *c) { * * It does a few things: * - * 1) Put the slave in ONLINE state. + * 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). * 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. @@ -660,7 +661,7 @@ void replconfCommand(redisClient *c) { void putSlaveOnline(redisClient *slave) { slave->replstate = REDIS_REPL_ONLINE; slave->repl_put_online_on_ack = 0; - slave->repl_ack_time = server.unixtime; + slave->repl_ack_time = server.unixtime; /* Prevent false timeout. */ if (aeCreateFileEvent(server.el, slave->fd, AE_WRITABLE, sendReplyToClient, slave) == AE_ERR) { redisLog(REDIS_WARNING,"Unable to register writable event for slave bulk transfer: %s", strerror(errno)); @@ -773,7 +774,7 @@ void updateSlavesWaitingBgsave(int bgsaveerr, int type) { * is technically online now. */ slave->replstate = REDIS_REPL_ONLINE; slave->repl_put_online_on_ack = 1; - slave->repl_ack_time = server.unixtime; + slave->repl_ack_time = server.unixtime; /* Timeout otherwise. */ } else { if (bgsaveerr != REDIS_OK) { freeClient(slave);