Introduce bgsave cancel

src/commands.def

@@ -6312,12 +6312,72 @@ struct COMMAND_STRUCT ACL_Subcommands[] = {
 #define BGREWRITEAOF_Keyspecs NULL
 #endif
 
+/********** BGSAVE CANCEL ********************/
+
+#ifndef SKIP_CMD_HISTORY_TABLE
+/* BGSAVE CANCEL history */
+#define BGSAVE_CANCEL_History NULL
+#endif
+
+#ifndef SKIP_CMD_TIPS_TABLE
+/* BGSAVE CANCEL tips */
+#define BGSAVE_CANCEL_Tips NULL
+#endif
+
+#ifndef SKIP_CMD_KEY_SPECS_TABLE
+/* BGSAVE CANCEL key specs */
+#define BGSAVE_CANCEL_Keyspecs NULL
+#endif
+
+/********** BGSAVE KILL ********************/
+
+#ifndef SKIP_CMD_HISTORY_TABLE
+/* BGSAVE KILL history */
+#define BGSAVE_KILL_History NULL
+#endif
+
+#ifndef SKIP_CMD_TIPS_TABLE
+/* BGSAVE KILL tips */
+#define BGSAVE_KILL_Tips NULL
+#endif
+
+#ifndef SKIP_CMD_KEY_SPECS_TABLE
+/* BGSAVE KILL key specs */
+#define BGSAVE_KILL_Keyspecs NULL
+#endif
+
+/********** BGSAVE SCHEDULE ********************/
+
+#ifndef SKIP_CMD_HISTORY_TABLE
+/* BGSAVE SCHEDULE history */
+#define BGSAVE_SCHEDULE_History NULL
+#endif
+
+#ifndef SKIP_CMD_TIPS_TABLE
+/* BGSAVE SCHEDULE tips */
+#define BGSAVE_SCHEDULE_Tips NULL
+#endif
+
+#ifndef SKIP_CMD_KEY_SPECS_TABLE
+/* BGSAVE SCHEDULE key specs */
+#define BGSAVE_SCHEDULE_Keyspecs NULL
+#endif
+
+/* BGSAVE command table */
+struct COMMAND_STRUCT BGSAVE_Subcommands[] = {
+{MAKE_CMD("cancel","Unschedule all scheduled bgsave requests.","O(1)","8.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,BGSAVE_CANCEL_History,0,BGSAVE_CANCEL_Tips,0,bgsaveCancelCommand,-1,CMD_ADMIN,0,BGSAVE_CANCEL_Keyspecs,0,NULL,0)},
+{MAKE_CMD("kill","Terminates a running bgsave child process.","O(1)","8.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,BGSAVE_KILL_History,0,BGSAVE_KILL_Tips,0,bgsaveKillCommand,-1,CMD_ADMIN,0,BGSAVE_KILL_Keyspecs,0,NULL,0)},
+{MAKE_CMD("schedule","Asynchronously saves the database(s) to disk. When an AOF rewrite is in progress and schedule the background save to run at the next opportunity.","O(1)","8.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,BGSAVE_SCHEDULE_History,0,BGSAVE_SCHEDULE_Tips,0,bgsaveScheduleCommand,-1,CMD_NO_ASYNC_LOADING|CMD_ADMIN|CMD_NOSCRIPT,0,BGSAVE_SCHEDULE_Keyspecs,0,NULL,0)},
+{0}
+};
+
 /********** BGSAVE ********************/
 
 #ifndef SKIP_CMD_HISTORY_TABLE
 /* BGSAVE history */
 commandHistory BGSAVE_History[] = {
 {"3.2.2","Added the `SCHEDULE` option."},
+{"8.0.0","Refactor SCHEDULE as a subcommand"},
 };
 #endif
 
@@ -6331,11 +6391,6 @@ commandHistory BGSAVE_History[] = {
 #define BGSAVE_Keyspecs NULL
 #endif
 
-/* BGSAVE argument table */
-struct COMMAND_ARG BGSAVE_Args[] = {
-{MAKE_ARG("schedule",ARG_TYPE_PURE_TOKEN,-1,"SCHEDULE",NULL,"3.2.2",CMD_ARG_OPTIONAL,0,NULL)},
-};
-
 /********** COMMAND COUNT ********************/
 
 #ifndef SKIP_CMD_HISTORY_TABLE
@@ -10899,7 +10954,7 @@ struct COMMAND_STRUCT serverCommandTable[] = {
 /* server */
 {MAKE_CMD("acl","A container for Access List Control commands.","Depends on subcommand.","6.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,ACL_History,0,ACL_Tips,0,NULL,-2,CMD_SENTINEL,0,ACL_Keyspecs,0,NULL,0),.subcommands=ACL_Subcommands},
 {MAKE_CMD("bgrewriteaof","Asynchronously rewrites the append-only file to disk.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,BGREWRITEAOF_History,0,BGREWRITEAOF_Tips,0,bgrewriteaofCommand,1,CMD_NO_ASYNC_LOADING|CMD_ADMIN|CMD_NOSCRIPT,0,BGREWRITEAOF_Keyspecs,0,NULL,0)},
-{MAKE_CMD("bgsave","Asynchronously saves the database(s) to disk.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,BGSAVE_History,1,BGSAVE_Tips,0,bgsaveCommand,-1,CMD_NO_ASYNC_LOADING|CMD_ADMIN|CMD_NOSCRIPT,0,BGSAVE_Keyspecs,0,NULL,1),.args=BGSAVE_Args},
+{MAKE_CMD("bgsave","Asynchronously saves the database(s) to disk.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,BGSAVE_History,2,BGSAVE_Tips,0,bgsaveCommand,-1,CMD_NO_ASYNC_LOADING|CMD_ADMIN|CMD_NOSCRIPT,0,BGSAVE_Keyspecs,0,NULL,0),.subcommands=BGSAVE_Subcommands},
 {MAKE_CMD("command","Returns detailed information about all commands.","O(N) where N is the total number of commands","2.8.13",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,COMMAND_History,0,COMMAND_Tips,1,commandCommand,-1,CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_CONNECTION,COMMAND_Keyspecs,0,NULL,0),.subcommands=COMMAND_Subcommands},
 {MAKE_CMD("config","A container for server configuration commands.","Depends on subcommand.","2.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,CONFIG_History,0,CONFIG_Tips,0,NULL,-2,0,0,CONFIG_Keyspecs,0,NULL,0),.subcommands=CONFIG_Subcommands},
 {MAKE_CMD("dbsize","Returns the number of keys in the database.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"server",COMMAND_GROUP_SERVER,DBSIZE_History,0,DBSIZE_Tips,2,dbsizeCommand,1,CMD_READONLY|CMD_FAST,ACL_CATEGORY_KEYSPACE,DBSIZE_Keyspecs,0,NULL,0)},

src/commands/bgsave-cancel.json

@@ -0,0 +1,21 @@
+{
+    "CANCEL": {
+        "summary": "Unschedule all scheduled bgsave requests.",
+        "complexity": "O(1)",
+        "group": "server",
+        "since": "8.0.0",
+        "arity": -1,
+        "container": "BGSAVE",
+        "function": "bgsaveCancelCommand",
+        "history": [
+        ],
+        "command_flags": [
+            "ADMIN"
+        ],
+        "arguments": [
+        ],
+        "reply_schema": {
+            "const": "OK"
+        }
+    }
+}

src/commands/bgsave-kill.json

@@ -0,0 +1,21 @@
+{
+    "KILL": {
+        "summary": "Terminates a running bgsave child process.",
+        "complexity": "O(1)",
+        "group": "server",
+        "since": "8.0.0",
+        "arity": -1,
+        "container": "BGSAVE",
+        "function": "bgsaveKillCommand",
+        "history": [
+        ],
+        "command_flags": [
+            "ADMIN"
+        ],
+        "arguments": [
+        ],
+        "reply_schema": {
+            "const": "OK"
+        }
+    }
+}

src/commands/bgsave-schedule.json

@@ -0,0 +1,23 @@
+{
+    "SCHEDULE": {
+        "summary": "Asynchronously saves the database(s) to disk. When an AOF rewrite is in progress and schedule the background save to run at the next opportunity.",
+        "complexity": "O(1)",
+        "group": "server",
+        "since": "8.0.0",
+        "arity": -1,
+        "container": "BGSAVE",
+        "function": "bgsaveScheduleCommand",
+        "history": [
+        ],
+        "command_flags": [
+            "NO_ASYNC_LOADING",
+            "ADMIN",
+            "NOSCRIPT"
+        ],
+        "arguments": [
+        ],
+        "reply_schema": {
+            "const": "Background saving scheduled"
+        }
+    }
+}

src/commands/bgsave.json

@@ -10,6 +10,10 @@
             [
                 "3.2.2",
                 "Added the `SCHEDULE` option."
+            ],
+            [
+                "8.0.0",
+                "Refactor SCHEDULE as a subcommand"
             ]
         ],
         "command_flags": [
@@ -18,23 +22,9 @@
             "NOSCRIPT"
         ],
         "arguments": [
-            {
-                "name": "schedule",
-                "token": "SCHEDULE",
-                "type": "pure-token",
-                "optional": true,
-                "since": "3.2.2"
-            }
         ],
         "reply_schema": {
-            "oneOf": [
-                {
-                    "const": "Background saving started"
-                },
-                {
-                    "const": "Background saving scheduled"
-                }
-            ]
+            "const": "Background saving started"
         }
     }
 }

src/rdb.c

@@ -3636,21 +3636,7 @@ void saveCommand(client *c) {
     }
 }
 
-/* BGSAVE [SCHEDULE] */
-void bgsaveCommand(client *c) {
-    int schedule = 0;
-
-    /* The SCHEDULE option changes the behavior of BGSAVE when an AOF rewrite
-     * is in progress. Instead of returning an error a BGSAVE gets scheduled. */
-    if (c->argc > 1) {
-        if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "schedule")) {
-            schedule = 1;
-        } else {
-            addReplyErrorObject(c, shared.syntaxerr);
-            return;
-        }
-    }
-
+void bgsaveCommandInternal(client *c, int schedule) {
     rdbSaveInfo rsi, *rsiptr;
     rsiptr = rdbPopulateSaveInfo(&rsi);
 
@@ -3672,6 +3658,42 @@ void bgsaveCommand(client *c) {
     }
 }
 
+/* BGSAVE KILL */
+void bgsaveKillCommand(client *c) {
+    /* Terminates an in progress BGSAVE */
+    if (server.child_type == CHILD_TYPE_RDB) {
+        /* There is an ongoing save */
+        serverLog(LL_NOTICE, "Background bgsave requested to be killed by user");
+        killRDBChild();
+        addReply(c, shared.ok);
+    } else {
+        addReplyError(c, "Background bgsave is currently not in progress");
+    }
+}
+
+/* BGSAVE CANCEL */
+void bgsaveCancelCommand(client *c) {
+    /* Cancel a scheduleds BGSAVE */
+    if (server.rdb_bgsave_scheduled == 1) {
+        /* There is an ongoing save */
+        serverLog(LL_NOTICE, "Background bgsave requested to be canceled by user");
+        server.rdb_bgsave_scheduled = 0;
+        addReply(c, shared.ok);
+    } else {
+        addReplyError(c, "Background bgsave is currently not scheduled");
+    }
+}
+
+/* BGSAVE SCHEDULE */
+void bgsaveScheduleCommand(client *c) {
+    bgsaveCommandInternal(c, 1);
+}
+
+/* BGSAVE COMMAND */
+void bgsaveCommand(client *c) {
+    bgsaveCommandInternal(c, 0);
+}
+
 /* Populate the rdbSaveInfo structure used to persist the replication
  * information inside the RDB file. Currently the structure explicitly
  * contains just the currently selected DB from the primary stream, however

src/server.h

@@ -3584,6 +3584,9 @@ void dbsizeCommand(client *c);
 void lastsaveCommand(client *c);
 void saveCommand(client *c);
 void bgsaveCommand(client *c);
+void bgsaveCancelCommand(client *c);
+void bgsaveKillCommand(client *c);
+void bgsaveScheduleCommand(client *c);
 void bgrewriteaofCommand(client *c);
 void shutdownCommand(client *c);
 void slowlogCommand(client *c);

tests/integration/rdb.tcl

@@ -170,6 +170,70 @@ start_server {} {
         }
         assert_equal [s rdb_changes_since_last_save] 0
     }
+
+    test {bgsave kill aborts save} {
+        r config set save ""
+        # Generating RDB will take some 100 seconds
+        r config set rdb-key-save-delay 1000000
+        populate 100 "" 16
+
+        r bgsave
+        wait_for_condition 50 100 {
+            [s rdb_bgsave_in_progress] == 1
+        } else {
+            fail "bgsave not done"
+        }
+        set fork_child_pid [get_child_pid 0]
+
+        assert {[r bgsave kill] eq {OK}}
+        set temp_rdb [file join [lindex [r config get dir] 1] temp-${fork_child_pid}.rdb]
+        # Temp rdb must be deleted
+        wait_for_condition 50 100 {
+            ![file exists $temp_rdb]
+        } else {
+            fail "bgsave temp file was not deleted after cancel"
+        }
+    }
+
+    test {bgsave kill response with error when no ongoing save} {
+        r config set save ""
+        # Generating RDB will take some 100 seconds
+        r config set rdb-key-save-delay 0
+
+        # Make sure no save is running and that bgsave return an error
+         wait_for_condition 50 100 {
+            [s rdb_bgsave_in_progress] == 0
+        } else {
+            fail "bgsave is currently running"
+        }
+        assert_error "ERR Background bgsave is currently not in progress" {r bgsave kill}
+    }
+
+    test {bgsave cancel request} {
+        r config set save ""
+        # Generating RDB will take some 100 seconds
+        r config set rdb-key-save-delay 1000000
+        populate 100 "" 16
+
+        # start a long AOF child
+        r bgrewriteaof
+        wait_for_condition 50 100 {
+            [s aof_rewrite_in_progress] == 1
+        } else {
+            fail "aof not started"
+        }
+
+        # Make sure cancel return valid status
+        assert {[r bgsave schedule] eq {Background saving scheduled}}
+
+        # Cancel the scheduled save
+        assert {[r bgsave cancel] eq {OK}}
+
+        # Make sure a second call to bgsave cancel return an error
+        assert_error "ERR Background bgsave is currently not scheduled" {r bgsave cancel}
+    }
+
+
 }
 
 test {client freed during loading} {