AOF refactoring, now with three states: ON, OFF, WAIT_REWRITE.

src/aof.c

@@ -19,14 +19,14 @@ void aof_background_fsync(int fd) {
 /* Called when the user switches from "appendonly yes" to "appendonly no"
  * at runtime using the CONFIG command. */
 void stopAppendOnly(void) {
+    redisAssert(server.aof_state != REDIS_AOF_OFF);
     flushAppendOnlyFile(1);
     aof_fsync(server.appendfd);
     close(server.appendfd);
 
     server.appendfd = -1;
     server.appendseldb = -1;
-    server.appendonly = 0;
+    server.aof_state = REDIS_AOF_OFF;
-    server.aof_wait_rewrite = 0;
     /* rewrite operation in progress? kill it, wait child exit */
     if (server.bgrewritechildpid != -1) {
         int statloc;
@@ -46,6 +46,7 @@ void stopAppendOnly(void) {
 int startAppendOnly(void) {
     server.lastfsync = time(NULL);
     server.appendfd = open(server.appendfilename,O_WRONLY|O_APPEND|O_CREAT,0644);
+    redisAssert(server.aof_state == REDIS_AOF_OFF);
     if (server.appendfd == -1) {
         redisLog(REDIS_WARNING,"Redis needs to enable the AOF but can't open the append only file: %s",strerror(errno));
         return REDIS_ERR;
@@ -57,8 +58,7 @@ int startAppendOnly(void) {
     }
     /* We correctly switched on AOF, now wait for the rerwite to be complete
      * in order to append data on disk. */
-    server.appendonly = 1;
+    server.aof_state = REDIS_AOF_WAIT_REWRITE;
-    server.aof_wait_rewrite = 1;
     return REDIS_OK;
 }
 
@@ -258,15 +258,9 @@ void feedAppendOnlyFile(struct redisCommand *cmd, int dictid, robj **argv, int a
 
     /* Append to the AOF buffer. This will be flushed on disk just before
      * of re-entering the event loop, so before the client will get a
-     * positive reply about the operation performed.
+     * positive reply about the operation performed. */
-     *
+    if (server.aof_state == REDIS_AOF_ON)
-     * Note, we don't add stuff in the AOF buffer if aof_wait_rewrite is
-     * non zero, as this means we are starting with a new AOF and the
-     * current one is meaningless (this happens for instance after
-     * a slave resyncs with its master). */
-    if (!server.aof_wait_rewrite) {
         server.aofbuf = sdscatlen(server.aofbuf,buf,sdslen(buf));
-    }
 
     /* If a background append only file rewriting is in progress we want to
      * accumulate the differences between the child DB and the current one
@@ -316,7 +310,7 @@ int loadAppendOnlyFile(char *filename) {
     struct redisClient *fakeClient;
     FILE *fp = fopen(filename,"r");
     struct redis_stat sb;
-    int appendonly = server.appendonly;
+    int old_aof_state = server.aof_state;
     long loops = 0;
 
     if (fp && redis_fstat(fileno(fp),&sb) != -1 && sb.st_size == 0) {
@@ -332,7 +326,7 @@ int loadAppendOnlyFile(char *filename) {
 
     /* Temporarily disable AOF, to prevent EXEC from feeding a MULTI
      * to the same file we're about to read. */
-    server.appendonly = 0;
+    server.aof_state = REDIS_AOF_OFF;
 
     fakeClient = createFakeClient();
     startLoading(fp);
@@ -401,7 +395,7 @@ int loadAppendOnlyFile(char *filename) {
 
     fclose(fp);
     freeFakeClient(fakeClient);
-    server.appendonly = appendonly;
+    server.aof_state = old_aof_state;
     stopLoading();
     aofUpdateCurrentSize();
     server.auto_aofrewrite_base_size = server.appendonly_current_size;
@@ -964,7 +958,9 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) {
         }
 
         redisLog(REDIS_NOTICE, "Background AOF rewrite successful");
-        server.aof_wait_rewrite = 0;
+        /* Change state from WAIT_REWRITE to ON if needed */
+        if (server.aof_state == REDIS_AOF_WAIT_REWRITE)
+            server.aof_state = REDIS_AOF_ON;
 
         /* Asynchronously close the overwritten AOF. */
         if (oldfd != -1) bioCreateBackgroundJob(REDIS_BIO_CLOSE_FILE,(void*)(long)oldfd,NULL,NULL);
@@ -984,10 +980,7 @@ cleanup:
     server.bgrewritebuf = sdsempty();
     aofRemoveTempFile(server.bgrewritechildpid);
     server.bgrewritechildpid = -1;
-    /* If we were waiting for an AOF rewrite before to start appending
+    /* Schedule a new rewrite if we are waiting for it to switch the AOF ON. */
-     * to the AOF again (this happens both when the user switches on
+    if (server.aof_state == REDIS_AOF_WAIT_REWRITE)
-     * AOF with CONFIG SET, and after a slave with AOF enabled syncs with
+        server.aofrewrite_scheduled = 1;
-     * 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;
 }

src/config.c

@@ -213,9 +213,12 @@ void loadServerConfigFromString(char *config) {
                 err = "argument must be 'yes' or 'no'"; goto loaderr;
             }
         } else if (!strcasecmp(argv[0],"appendonly") && argc == 2) {
-            if ((server.appendonly = yesnotoi(argv[1])) == -1) {
+            int yes;
+
+            if ((yes = yesnotoi(argv[1])) == -1) {
                 err = "argument must be 'yes' or 'no'"; goto loaderr;
             }
+            server.aof_state = yes ? REDIS_AOF_ON : REDIS_AOF_OFF;
         } else if (!strcasecmp(argv[0],"appendfilename") && argc == 2) {
             zfree(server.appendfilename);
             server.appendfilename = zstrdup(argv[1]);
@@ -426,19 +429,16 @@ void configSetCommand(redisClient *c) {
         if (yn == -1) goto badfmt;
         server.no_appendfsync_on_rewrite = yn;
     } else if (!strcasecmp(c->argv[2]->ptr,"appendonly")) {
-        int old = server.appendonly;
+        int enable = yesnotoi(o->ptr);
-        int new = yesnotoi(o->ptr);
 
-        if (new == -1) goto badfmt;
+        if (enable == -1) goto badfmt;
-        if (old != new) {
+        if (enable == 0 && server.aof_state != REDIS_AOF_OFF) {
-            if (new == 0) {
+            stopAppendOnly();
-                stopAppendOnly();
+        } else if (enable && server.aof_state == REDIS_AOF_OFF) {
-            } else {
+            if (startAppendOnly() == REDIS_ERR) {
-                if (startAppendOnly() == REDIS_ERR) {
+                addReplyError(c,
-                    addReplyError(c,
+                    "Unable to turn on AOF. Check server logs.");
-                        "Unable to turn on AOF. Check server logs.");
+                return;
-                    return;
-                }
             }
         }
     } else if (!strcasecmp(c->argv[2]->ptr,"auto-aof-rewrite-percentage")) {
@@ -616,7 +616,7 @@ void configGetCommand(redisClient *c) {
     }
     if (stringmatch(pattern,"appendonly",0)) {
         addReplyBulkCString(c,"appendonly");
-        addReplyBulkCString(c,server.appendonly ? "yes" : "no");
+        addReplyBulkCString(c,server.aof_state == REDIS_AOF_OFF ? "no" : "yes");
         matches++;
     }
     if (stringmatch(pattern,"no-appendfsync-on-rewrite",0)) {

src/db.c

@@ -487,7 +487,7 @@ void propagateExpire(redisDb *db, robj *key) {
     argv[1] = key;
     incrRefCount(key);
 
-    if (server.appendonly)
+    if (server.aof_state != REDIS_AOF_OFF)
         feedAppendOnlyFile(server.delCommand,db->id,argv,2);
     if (listLength(server.slaves))
         replicationFeedSlaves(server.slaves,db->id,argv,2);

src/multi.c

@@ -67,7 +67,7 @@ void discardCommand(redisClient *c) {
 void execCommandReplicateMulti(redisClient *c) {
     robj *multistring = createStringObject("MULTI",5);
 
-    if (server.appendonly)
+    if (server.aof_state != REDIS_AOF_OFF)
         feedAppendOnlyFile(server.multiCommand,c->db->id,&multistring,1);
     if (listLength(server.slaves))
         replicationFeedSlaves(server.slaves,c->db->id,&multistring,1);

src/redis.c

@@ -873,14 +873,13 @@ void initServerConfig() {
     server.syslog_ident = zstrdup("redis");
     server.syslog_facility = LOG_LOCAL0;
     server.daemonize = 0;
-    server.appendonly = 0;
+    server.aof_state = REDIS_AOF_OFF;
     server.appendfsync = APPENDFSYNC_EVERYSEC;
     server.no_appendfsync_on_rewrite = 0;
     server.auto_aofrewrite_perc = REDIS_AUTO_AOFREWRITE_PERC;
     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 */
@@ -920,7 +919,6 @@ void initServerConfig() {
     appendServerSaveParams(300,100);  /* save after 5 minutes and 100 changes */
     appendServerSaveParams(60,10000); /* save after 1 minute and 10000 changes */
     /* Replication related */
-    server.isslave = 0;
     server.masterauth = NULL;
     server.masterhost = NULL;
     server.masterport = 6379;
@@ -1069,8 +1067,9 @@ void initServer() {
     if (server.sofd > 0 && aeCreateFileEvent(server.el,server.sofd,AE_READABLE,
         acceptUnixHandler,NULL) == AE_ERR) oom("creating file event");
 
-    if (server.appendonly) {
+    if (server.aof_state == REDIS_AOF_ON) {
-        server.appendfd = open(server.appendfilename,O_WRONLY|O_APPEND|O_CREAT,0644);
+        server.appendfd = open(server.appendfilename,
+                               O_WRONLY|O_APPEND|O_CREAT,0644);
         if (server.appendfd == -1) {
             redisLog(REDIS_WARNING, "Can't open the append-only file: %s",
                 strerror(errno));
@@ -1156,7 +1155,7 @@ void call(redisClient *c) {
     slowlogPushEntryIfNeeded(c->argv,c->argc,duration);
     c->cmd->calls++;
 
-    if (server.appendonly && dirty > 0)
+    if (server.aof_state != REDIS_AOF_OFF && dirty > 0)
         feedAppendOnlyFile(c->cmd,c->db->id,c->argv,c->argc);
     if ((dirty > 0 || c->cmd->flags & REDIS_CMD_FORCE_REPLICATION) &&
         listLength(server.slaves))
@@ -1311,7 +1310,7 @@ int prepareForShutdown(int flags) {
         kill(server.bgsavechildpid,SIGKILL);
         rdbRemoveTempFile(server.bgsavechildpid);
     }
-    if (server.appendonly) {
+    if (server.aof_state != REDIS_AOF_OFF) {
         /* Kill the AOF saving child as the AOF we already have may be longer
          * but contains the full dataset anyway. */
         if (server.bgrewritechildpid != -1) {
@@ -1498,13 +1497,13 @@ sds genRedisInfoString(char *section) {
             "last_save_time:%ld\r\n"
             "bgrewriteaof_in_progress:%d\r\n",
             server.loading,
-            server.appendonly,
+            server.aof_state != REDIS_AOF_OFF,
             server.dirty,
             server.bgsavechildpid != -1,
             server.lastsave,
             server.bgrewritechildpid != -1);
 
-        if (server.appendonly) {
+        if (server.aof_state != REDIS_AOF_OFF) {
             info = sdscatprintf(info,
                 "aof_current_size:%lld\r\n"
                 "aof_base_size:%lld\r\n"
@@ -2099,7 +2098,7 @@ int main(int argc, char **argv) {
     linuxOvercommitMemoryWarning();
 #endif
     start = ustime();
-    if (server.appendonly) {
+    if (server.aof_state == REDIS_AOF_ON) {
         if (loadAppendOnlyFile(server.appendfilename) == REDIS_OK)
             redisLog(REDIS_NOTICE,"DB loaded from append only file: %.3f seconds",(float)(ustime()-start)/1000000);
     } else {

src/redis.h

@@ -560,7 +560,6 @@ 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 */
     pid_t bgrewritechildpid;        /* PID if rewriting process */
     sds bgrewritebuf; /* buffer taken by parent during oppend only rewrite */
     sds aofbuf;       /* AOF buffer, written before entering the event loop */

src/replication.c

@@ -361,7 +361,7 @@ void readSyncBulkPayload(aeEventLoop *el, int fd, void *privdata, int mask) {
         /* 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) {
+        if (server.aof_state != REDIS_AOF_OFF) {
             int retry = 10;
 
             stopAppendOnly();