From e7a2e7c1f7b1e46704aaf0850f75d13d418c1db7 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 15 Dec 2011 16:07:49 +0100 Subject: [PATCH] AOF fixes in the context of replicaiton (when AOF is used by slave) and CONFIG SET appendonly yes/no. --- src/aof.c | 24 +++++++++++++++++++++--- src/redis.c | 1 + src/redis.h | 1 + src/replication.c | 18 ++++++++++++++++-- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/aof.c b/src/aof.c index bfa8163d..4d342091 100644 --- a/src/aof.c +++ b/src/aof.c @@ -26,6 +26,7 @@ void stopAppendOnly(void) { server.appendfd = -1; server.appendseldb = -1; server.appendonly = 0; + server.aof_wait_rewrite = 0; /* rewrite operation in progress? kill it, wait child exit */ if (server.bgrewritechildpid != -1) { int statloc; @@ -35,6 +36,7 @@ void stopAppendOnly(void) { /* reset the buffer accumulating changes while the child saves */ sdsfree(server.bgrewritebuf); server.bgrewritebuf = sdsempty(); + aofRemoveTempFile(server.bgrewritechildpid); server.bgrewritechildpid = -1; } } @@ -46,15 +48,18 @@ int startAppendOnly(void) { server.lastfsync = time(NULL); server.appendfd = open(server.appendfilename,O_WRONLY|O_APPEND|O_CREAT,0644); if (server.appendfd == -1) { - redisLog(REDIS_WARNING,"Used tried to switch on AOF via CONFIG, but I can't open the AOF file: %s",strerror(errno)); + redisLog(REDIS_WARNING,"Redis needs to enable the AOF but can't open the append only file: %s",strerror(errno)); return REDIS_ERR; } if (rewriteAppendOnlyFileBackground() == REDIS_ERR) { server.appendonly = 0; close(server.appendfd); - redisLog(REDIS_WARNING,"User tried turning on AOF with CONFIG SET but I can't trigger a background AOF rewrite operation. Check the above logs for more info about the error."); + redisLog(REDIS_WARNING,"Redis needs to enable the AOF but can't trigger a background AOF rewrite operation. Check the above logs for more info about the error."); return REDIS_ERR; } + /* We correctly switched on AOF, now wait for the rerwite to be complete + * in order to append data on disk. */ + server.aof_wait_rewrite = 1; return REDIS_OK; } @@ -219,9 +224,15 @@ sds catAppendOnlyExpireAtCommand(sds buf, struct redisCommand *cmd, robj *key, r } void feedAppendOnlyFile(struct redisCommand *cmd, int dictid, robj **argv, int argc) { - sds buf = sdsempty(); + sds buf; robj *tmpargv[3]; + /* Return ASAP if we are writing a rewrite to finish in order to start + * appending to the Append Only File. */ + if (server.aof_wait_rewrite) return; + + buf = sdsempty(); + /* The DB this command was targetting is not the same as the last command * we appendend. To issue a SELECT command is needed. */ if (dictid != server.appendseldb) { @@ -953,6 +964,7 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) { } redisLog(REDIS_NOTICE, "Background AOF rewrite successful"); + server.aof_wait_rewrite = 0; /* Asynchronously close the overwritten AOF. */ if (oldfd != -1) bioCreateBackgroundJob(REDIS_BIO_CLOSE_FILE,(void*)(long)oldfd,NULL,NULL); @@ -972,4 +984,10 @@ cleanup: server.bgrewritebuf = sdsempty(); aofRemoveTempFile(server.bgrewritechildpid); server.bgrewritechildpid = -1; + /* If we were waiting for an AOF rewrite before to start appending + * to the AOF again (this happens both when the user switches on + * AOF with CONFIG SET, and after a slave with AOF enabled syncs with + * the master), but the rewrite failed (otherwise aof_wait_rewrite + * would be zero), we need to schedule a new one. */ + if (server.aof_wait_rewrite) server.aofrewrite_scheduled = 1; } diff --git a/src/redis.c b/src/redis.c index 28240b03..f75aa791 100644 --- a/src/redis.c +++ b/src/redis.c @@ -880,6 +880,7 @@ void initServerConfig() { server.auto_aofrewrite_min_size = REDIS_AUTO_AOFREWRITE_MIN_SIZE; server.auto_aofrewrite_base_size = 0; server.aofrewrite_scheduled = 0; + server.aof_wait_rewrite = 0; server.lastfsync = time(NULL); server.appendfd = -1; server.appendseldb = -1; /* Make sure the first time will not match */ diff --git a/src/redis.h b/src/redis.h index 6b9bad08..79a28911 100644 --- a/src/redis.h +++ b/src/redis.h @@ -547,6 +547,7 @@ struct redisServer { off_t auto_aofrewrite_base_size;/* AOF size on latest startup or rewrite. */ off_t appendonly_current_size; /* AOF current size. */ int aofrewrite_scheduled; /* Rewrite once BGSAVE terminates. */ + int aof_wait_rewrite; /* Don't append to AOF before rewrite */ int shutdown_asap; /* SHUTDOWN needed */ int activerehashing; char *requirepass; diff --git a/src/replication.c b/src/replication.c index 30df9bc8..2be03c1b 100644 --- a/src/replication.c +++ b/src/replication.c @@ -358,8 +358,22 @@ void readSyncBulkPayload(aeEventLoop *el, int fd, void *privdata, int mask) { server.master->authenticated = 1; server.replstate = REDIS_REPL_CONNECTED; redisLog(REDIS_NOTICE, "MASTER <-> SLAVE sync: Finished with success"); - /* Rewrite the AOF file now that the dataset changed. */ - if (server.appendonly) rewriteAppendOnlyFileBackground(); + /* Restart the AOF subsystem now that we finished the sync. This + * will trigger an AOF rewrite, and when done will start appending + * to the new file. */ + if (server.appendonly) { + int retry = 10; + + stopAppendOnly(); + while (retry-- && startAppendOnly() == REDIS_ERR) { + redisLog(REDIS_WARNING,"Failed enabling the AOF after successful master synchrnization! Trying it again in one second."); + sleep(1); + } + if (!retry) { + redisLog(REDIS_WARNING,"FATAL: this slave instance finished the synchronization with its master, but the AOF can't be turned on. Exiting now."); + exit(1); + } + } } return;