Kill the background saving process before performing SHUTDOWN to avoid races

diff --git a/benchmark.c b/benchmark.c
index 7d4c844d5218a82742ff272c74686d50f7f583f6..0d53c935aec3ebbe07d8fcb6266b8f543d77fd52 100644 (file)
--- a/benchmark.c
+++ b/benchmark.c
@@ -425,15 +425,6 @@ int main(int argc, char **argv) {
     }
 
     do {
-        prepareForBenchmark();
-        c = createClient();
-        if (!c) exit(1);
-        c->obuf = sdscat(c->obuf,"PING\r\n");
-        c->replytype = REPLY_RETCODE;
-        createMissingClients(c);
-        aeMain(config.el);
-        endBenchmark("PING");
-
         prepareForBenchmark();
         c = createClient();
         if (!c) exit(1);
@@ -488,6 +479,15 @@ int main(int argc, char **argv) {
         aeMain(config.el);
         endBenchmark("LPOP");
 
+        prepareForBenchmark();
+        c = createClient();
+        if (!c) exit(1);
+        c->obuf = sdscat(c->obuf,"PING\r\n");
+        c->replytype = REPLY_RETCODE;
+        createMissingClients(c);
+        aeMain(config.el);
+        endBenchmark("PING");
+
         printf("\n");
     } while(config.loop);
 

diff --git a/doc/LremCommand.html b/doc/LremCommand.html
index f0585ef7c51bc9eaf54cb8bef7f61b55bcbd24ed..c7d6c8785eab2df5645e10c48ea857dcb1d46267 100644 (file)
--- a/doc/LremCommand.html
+++ b/doc/LremCommand.html
@@ -27,7 +27,7 @@
 
                 <div class="narrow">
                     <h1><a name="LREM _key_ _count_ _value_">LREM _key_ _count_ _value_</a></h1>
-<i>Time complexity: O(N) (with N being the length of the list)</i><blockquote>Remove the first <i>count</i> occurrences of the <i>value</i> element from the list.If <i>count</i> is zero all the elements are removed. If <i>count</i> is negativeelements are removed from tail to head, instead to go from head to tailthat is the normal behaviour. So for example LREM with count -2 and_hello_ as value to remove against the list (a,b,c,hello,x,hello,hello) willlave the list (a,b,c,hello,x). The number of removed elements is returnedas an integer, see below for more information aboht the returned value.</blockquote>
+<i>Time complexity: O(N) (with N being the length of the list)</i><blockquote>Remove the first <i>count</i> occurrences of the <i>value</i> element from the list.If <i>count</i> is zero all the elements are removed. If <i>count</i> is negativeelements are removed from tail to head, instead to go from head to tailthat is the normal behaviour. So for example LREM with count -2 and_hello_ as value to remove against the list (a,b,c,hello,x,hello,hello) willlave the list (a,b,c,hello,x). The number of removed elements is returnedas an integer, see below for more information about the returned value.Note that non existing keys are considered like empty lists by LREM, so LREMagainst non existing keys will always return 0.</blockquote>
 <h2><a name="Return value">Return value</a></h2><a href="ReplyTypes.html">Integer Reply</a>, specifically:<br/><br/><pre class="codeblock python" name="code">
 The number of removed elements if the operation succeeded
 </pre><h2><a name="See also">See also</a></h2>

diff --git a/redis.c b/redis.c
index 8b3f3be24a228ecd24f58d4196fbe7618306cb5b..ff8db0be47c832551577b74221f3d6f955bdd2c8 100644 (file)
--- a/redis.c
+++ b/redis.c
@@ -240,6 +240,7 @@ struct redisServer {
     int daemonize;
     char *pidfile;
     int bgsaveinprogress;
+    pid_t bgsavechildpid;
     struct saveparam *saveparams;
     int saveparamslen;
     char *logfile;
@@ -748,16 +749,21 @@ int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) {
         /* XXX: TODO handle the case of the saving child killed */
         if (wait4(-1,&statloc,WNOHANG,NULL)) {
             int exitcode = WEXITSTATUS(statloc);
-            if (exitcode == 0) {
+            int bysignal = WIFSIGNALED(statloc);
+
+            if (!bysignal && exitcode == 0) {
                 redisLog(REDIS_NOTICE,
                     "Background saving terminated with success");
                 server.dirty = 0;
                 server.lastsave = time(NULL);
+            } else if (!bysignal && exitcode != 0) {
+                redisLog(REDIS_WARNING, "Background saving error");
             } else {
                 redisLog(REDIS_WARNING,
-                    "Background saving error");
+                    "Background saving terminated by signal");
             }
             server.bgsaveinprogress = 0;
+            server.bgsavechildpid = -1;
             updateSalvesWaitingBgsave(exitcode == 0 ? REDIS_OK : REDIS_ERR);
         }
     } else {
@@ -919,6 +925,7 @@ static void initServer() {
     }
     server.cronloops = 0;
     server.bgsaveinprogress = 0;
+    server.bgsavechildpid = -1;
     server.lastsave = time(NULL);
     server.dirty = 0;
     server.usedmemory = 0;
@@ -1983,6 +1990,7 @@ static int rdbSaveBackground(char *filename) {
         }
         redisLog(REDIS_NOTICE,"Background saving started by pid %d",childpid);
         server.bgsaveinprogress = 1;
+        server.bgsavechildpid = childpid;
         return REDIS_OK;
     }
     return REDIS_OK; /* unreached */
@@ -2505,11 +2513,13 @@ static void bgsaveCommand(redisClient *c) {
 
 static void shutdownCommand(redisClient *c) {
     redisLog(REDIS_WARNING,"User requested shutdown, saving DB...");
-    /* XXX: TODO kill the child if there is a bgsave in progress */
+    if (server.bgsaveinprogress) {
+        redisLog(REDIS_WARNING,"There is a live saving child. Killing it!");
+        kill(server.bgsavechildpid,SIGKILL);
+    }
     if (rdbSave(server.dbfilename) == REDIS_OK) {
-        if (server.daemonize) {
+        if (server.daemonize)
             unlink(server.pidfile);
-        }
         redisLog(REDIS_WARNING,"%zu bytes used at exit",zmalloc_used_memory());
         redisLog(REDIS_WARNING,"Server exit now, bye bye...");
         exit(1);