From bba9bfc91c64893405de6db5162ceecc4552c37f Mon Sep 17 00:00:00 2001 From: ranshid Date: Mon, 8 Jul 2024 11:53:34 +0000 Subject: [PATCH 01/32] Introduce bgsave cancel In some cases bgsave child process can run for a long time exhausting system resources. Although it is possible to kill the bgsave child process from the system shell, sometimes it is not possible allowing OS level access. This bgsave extention provides an option to cancel the currently running bgsave process. Signed-off-by: ranshid --- src/rdb.c | 12 ++++++++++++ tests/integration/rdb.tcl | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/rdb.c b/src/rdb.c index 8b1037ab93..7d0b8e9186 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3645,6 +3645,18 @@ void bgsaveCommand(client *c) { if (c->argc > 1) { if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "schedule")) { schedule = 1; + } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "cancel")) { + /* BGSAVE CANCEL + * Terminates an in progress BGSAVE */ + if (server.child_type == CHILD_TYPE_RDB) { + /* There is an ongoing save */ + serverLog(LL_NOTICE, "Background save requested to be canceled by user"); + killRDBChild(); + addReply(c, shared.ok); + } else { + addReplyError(c, "background save is currently not in progress"); + } + return; } else { addReplyErrorObject(c, shared.syntaxerr); return; diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index e3f92bf521..4802512775 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -170,6 +170,46 @@ start_server {} { } assert_equal [s rdb_changes_since_last_save] 0 } + + test {bgsave cancel 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 cancel] eq {OK}} + set temp_rdb [file join [lindex [r config get dir] 1] temp-${fork_child_pid}.rdb] + # Temp rdb must be existed + wait_for_condition 50 100 { + ![file exists $temp_rdb] + } else { + fail "bgsave temp file was not deleted after cancel" + } + } + + test {bgsave cancel 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 save is currently not in progress" {r bgsave cancel} + } + + } test {client freed during loading} { From 09ac83599e740d597d12d3445cefc28d06bf3acc Mon Sep 17 00:00:00 2001 From: ranshid Date: Mon, 8 Jul 2024 14:03:55 +0000 Subject: [PATCH 02/32] clang format fix Signed-off-by: ranshid --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index 7d0b8e9186..20d4aff7aa 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3647,7 +3647,7 @@ void bgsaveCommand(client *c) { schedule = 1; } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "cancel")) { /* BGSAVE CANCEL - * Terminates an in progress BGSAVE */ + * Terminates an in progress BGSAVE */ if (server.child_type == CHILD_TYPE_RDB) { /* There is an ongoing save */ serverLog(LL_NOTICE, "Background save requested to be canceled by user"); From 7899fad1e682e59dae76a3318c6abb41ec52e4a9 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Tue, 9 Jul 2024 09:55:53 +0300 Subject: [PATCH 03/32] Update src/rdb.c Co-authored-by: Binbin Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index 20d4aff7aa..ea30b8c32b 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3654,7 +3654,7 @@ void bgsaveCommand(client *c) { killRDBChild(); addReply(c, shared.ok); } else { - addReplyError(c, "background save is currently not in progress"); + addReplyError(c, "Background save is currently not in progress"); } return; } else { From 0572c7a39f029e9e761858f130fc45109bffd928 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Tue, 9 Jul 2024 09:56:02 +0300 Subject: [PATCH 04/32] Update src/rdb.c Co-authored-by: Binbin Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index ea30b8c32b..e331d9f74d 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3649,7 +3649,7 @@ void bgsaveCommand(client *c) { /* BGSAVE CANCEL * Terminates an in progress BGSAVE */ if (server.child_type == CHILD_TYPE_RDB) { - /* There is an ongoing save */ + /* There is an ongoing bgsave */ serverLog(LL_NOTICE, "Background save requested to be canceled by user"); killRDBChild(); addReply(c, shared.ok); From c25a85b8f3cc91a02b6a2c0b2281b1af1201b96e Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Tue, 9 Jul 2024 09:56:19 +0300 Subject: [PATCH 05/32] Update tests/integration/rdb.tcl Co-authored-by: Binbin Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- tests/integration/rdb.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index 4802512775..cf4f670aa5 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -187,7 +187,7 @@ start_server {} { assert {[r bgsave cancel] eq {OK}} set temp_rdb [file join [lindex [r config get dir] 1] temp-${fork_child_pid}.rdb] - # Temp rdb must be existed + # Temp rdb must be deleted wait_for_condition 50 100 { ![file exists $temp_rdb] } else { From 9f7f3aa0e1e278ff027ee0935c11070674f12016 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Tue, 9 Jul 2024 09:56:27 +0300 Subject: [PATCH 06/32] Update tests/integration/rdb.tcl Co-authored-by: Binbin Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- tests/integration/rdb.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index cf4f670aa5..2456a38be5 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -206,7 +206,7 @@ start_server {} { } else { fail "bgsave is currently running" } - assert_error "ERR background save is currently not in progress" {r bgsave cancel} + assert_error "ERR Background save is currently not in progress" {r bgsave cancel} } From 50bc3f0548603a6c8aaaa916354cf1967d41dae9 Mon Sep 17 00:00:00 2001 From: ranshid Date: Tue, 9 Jul 2024 09:44:56 +0000 Subject: [PATCH 07/32] change bgsave cancel to be a sub-command Signed-off-by: ranshid --- src/commands.def | 25 ++++++++++++++++++++++++- src/commands/bgsave-cancel.json | 24 ++++++++++++++++++++++++ src/rdb.c | 17 +++++++++++++++++ src/server.h | 1 + 4 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 src/commands/bgsave-cancel.json diff --git a/src/commands.def b/src/commands.def index 4559c0aefe..aebc5087e1 100644 --- a/src/commands.def +++ b/src/commands.def @@ -6312,6 +6312,29 @@ 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 command table */ +struct COMMAND_STRUCT BGSAVE_Subcommands[] = { +{MAKE_CMD("cancel","Terminates a running bgsave child process.","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,ACL_CATEGORY_CONNECTION,BGSAVE_CANCEL_Keyspecs,0,NULL,0)}, +{0} +}; + /********** BGSAVE ********************/ #ifndef SKIP_CMD_HISTORY_TABLE @@ -10899,7 +10922,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,1,BGSAVE_Tips,0,bgsaveCommand,-1,CMD_NO_ASYNC_LOADING|CMD_ADMIN|CMD_NOSCRIPT,0,BGSAVE_Keyspecs,0,NULL,1),.subcommands=BGSAVE_Subcommands,.args=BGSAVE_Args}, {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)}, diff --git a/src/commands/bgsave-cancel.json b/src/commands/bgsave-cancel.json new file mode 100644 index 0000000000..8a03af0bcc --- /dev/null +++ b/src/commands/bgsave-cancel.json @@ -0,0 +1,24 @@ +{ + "CANCEL": { + "summary": "Terminates a running bgsave child process.", + "complexity": "O(1)", + "group": "server", + "since": "8.0.0", + "arity": -1, + "container": "BGSAVE", + "function": "bgsaveCancelCommand", + "history": [ + ], + "command_flags": [ + "ADMIN" + ], + "acl_categories": [ + "CONNECTION" + ], + "arguments": [ + ], + "reply_schema": { + "const": "OK" + } + } +} diff --git a/src/rdb.c b/src/rdb.c index e331d9f74d..ac000fcdf6 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3636,6 +3636,20 @@ void saveCommand(client *c) { } } +/* BGSAVE CANCEL */ +void bgsaveCancelCommand(client *c) { + /* BGSAVE CANCEL + * Terminates an in progress BGSAVE */ + if (server.child_type == CHILD_TYPE_RDB) { + /* There is an ongoing save */ + serverLog(LL_NOTICE, "Background save requested to be canceled by user"); + killRDBChild(); + addReply(c, shared.ok); + } else { + addReplyError(c, "background save is currently not in progress"); + } +} + /* BGSAVE [SCHEDULE] */ void bgsaveCommand(client *c) { int schedule = 0; @@ -3645,6 +3659,7 @@ void bgsaveCommand(client *c) { if (c->argc > 1) { if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "schedule")) { schedule = 1; +<<<<<<< Updated upstream } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "cancel")) { /* BGSAVE CANCEL * Terminates an in progress BGSAVE */ @@ -3657,6 +3672,8 @@ void bgsaveCommand(client *c) { addReplyError(c, "Background save is currently not in progress"); } return; +======= +>>>>>>> Stashed changes } else { addReplyErrorObject(c, shared.syntaxerr); return; diff --git a/src/server.h b/src/server.h index 73f68a73d4..d5020dc596 100644 --- a/src/server.h +++ b/src/server.h @@ -3584,6 +3584,7 @@ void dbsizeCommand(client *c); void lastsaveCommand(client *c); void saveCommand(client *c); void bgsaveCommand(client *c); +void bgsaveCancelCommand(client *c); void bgrewriteaofCommand(client *c); void shutdownCommand(client *c); void slowlogCommand(client *c); From 8aff7ac62a4d8021ce05159a444bc2cc95b49384 Mon Sep 17 00:00:00 2001 From: ranshid Date: Tue, 9 Jul 2024 09:49:07 +0000 Subject: [PATCH 08/32] fix bad merge-stash Signed-off-by: ranshid --- src/rdb.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index ac000fcdf6..36bb0afd95 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3659,21 +3659,6 @@ void bgsaveCommand(client *c) { if (c->argc > 1) { if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "schedule")) { schedule = 1; -<<<<<<< Updated upstream - } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "cancel")) { - /* BGSAVE CANCEL - * Terminates an in progress BGSAVE */ - if (server.child_type == CHILD_TYPE_RDB) { - /* There is an ongoing bgsave */ - serverLog(LL_NOTICE, "Background save requested to be canceled by user"); - killRDBChild(); - addReply(c, shared.ok); - } else { - addReplyError(c, "Background save is currently not in progress"); - } - return; -======= ->>>>>>> Stashed changes } else { addReplyErrorObject(c, shared.syntaxerr); return; From c7b71ecc6df1fc2584daec51033eb1e2c2cc02ad Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Tue, 9 Jul 2024 12:50:32 +0300 Subject: [PATCH 09/32] Update src/rdb.c Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index 36bb0afd95..ddf372ddc8 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3646,7 +3646,7 @@ void bgsaveCancelCommand(client *c) { killRDBChild(); addReply(c, shared.ok); } else { - addReplyError(c, "background save is currently not in progress"); + addReplyError(c, "Background bgsave is currently not in progress"); } } From 6b823e328221bed00a5a82e70d135a0f91619869 Mon Sep 17 00:00:00 2001 From: ranshid Date: Tue, 9 Jul 2024 09:58:00 +0000 Subject: [PATCH 10/32] some minor extra fixes Signed-off-by: ranshid --- src/commands/bgsave-cancel.json | 3 --- src/rdb.c | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/commands/bgsave-cancel.json b/src/commands/bgsave-cancel.json index 8a03af0bcc..b91b6d8427 100644 --- a/src/commands/bgsave-cancel.json +++ b/src/commands/bgsave-cancel.json @@ -12,9 +12,6 @@ "command_flags": [ "ADMIN" ], - "acl_categories": [ - "CONNECTION" - ], "arguments": [ ], "reply_schema": { diff --git a/src/rdb.c b/src/rdb.c index ddf372ddc8..910f653008 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3642,7 +3642,7 @@ void bgsaveCancelCommand(client *c) { * Terminates an in progress BGSAVE */ if (server.child_type == CHILD_TYPE_RDB) { /* There is an ongoing save */ - serverLog(LL_NOTICE, "Background save requested to be canceled by user"); + serverLog(LL_NOTICE, "Background bgsave requested to be canceled by user"); killRDBChild(); addReply(c, shared.ok); } else { From 63f95290bb74982eb01b629ddda03a6e4e507256 Mon Sep 17 00:00:00 2001 From: ranshid Date: Tue, 9 Jul 2024 10:01:49 +0000 Subject: [PATCH 11/32] fix rdb.tcl after error string change Signed-off-by: ranshid --- src/commands.def | 2 +- tests/integration/rdb.tcl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands.def b/src/commands.def index aebc5087e1..2a2ac24f61 100644 --- a/src/commands.def +++ b/src/commands.def @@ -6331,7 +6331,7 @@ struct COMMAND_STRUCT ACL_Subcommands[] = { /* BGSAVE command table */ struct COMMAND_STRUCT BGSAVE_Subcommands[] = { -{MAKE_CMD("cancel","Terminates a running bgsave child process.","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,ACL_CATEGORY_CONNECTION,BGSAVE_CANCEL_Keyspecs,0,NULL,0)}, +{MAKE_CMD("cancel","Terminates a running bgsave child process.","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)}, {0} }; diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index 2456a38be5..f1c8129b4a 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -206,7 +206,7 @@ start_server {} { } else { fail "bgsave is currently running" } - assert_error "ERR Background save is currently not in progress" {r bgsave cancel} + assert_error "ERR Background bgsave is currently not in progress" {r bgsave cancel} } From 1878f865464114ff355a6696ed6b5e4de7a266ce Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Wed, 10 Jul 2024 10:49:01 +0300 Subject: [PATCH 12/32] Update src/rdb.c Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index 910f653008..13f3a3e4ea 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3638,8 +3638,7 @@ void saveCommand(client *c) { /* BGSAVE CANCEL */ void bgsaveCancelCommand(client *c) { - /* BGSAVE CANCEL - * Terminates an in progress BGSAVE */ + /* 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 canceled by user"); From 528038fa2a290e0fd6093c9ec8d16341629cd50f Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Thu, 18 Jul 2024 19:18:20 +0300 Subject: [PATCH 13/32] 1. change bgsave schedule to be a sub-command 2. introduce bgsave cancel sub command 3. introduce bgsave kill sub command 4. add test for bgsave cancel Signed-off-by: Ran Shidlansik --- src/commands.def | 46 ++++++++++++++++++---- src/commands/bgsave-cancel.json | 2 +- src/commands/bgsave-kill.json | 21 ++++++++++ src/commands/bgsave-schedule.json | 23 +++++++++++ src/commands/bgsave.json | 20 +++------- src/rdb.c | 64 ++++++++++++++++++------------- src/server.h | 2 + tests/integration/rdb.tcl | 32 ++++++++++++++-- 8 files changed, 156 insertions(+), 54 deletions(-) create mode 100644 src/commands/bgsave-kill.json create mode 100644 src/commands/bgsave-schedule.json diff --git a/src/commands.def b/src/commands.def index 2a2ac24f61..96b653859c 100644 --- a/src/commands.def +++ b/src/commands.def @@ -6329,9 +6329,45 @@ struct COMMAND_STRUCT ACL_Subcommands[] = { #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","Terminates a running bgsave child process.","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("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} }; @@ -6341,6 +6377,7 @@ struct COMMAND_STRUCT BGSAVE_Subcommands[] = { /* BGSAVE history */ commandHistory BGSAVE_History[] = { {"3.2.2","Added the `SCHEDULE` option."}, +{"8.0.0","Refactor SCHEDULE as a subcommand"}, }; #endif @@ -6354,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 @@ -10922,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),.subcommands=BGSAVE_Subcommands,.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)}, diff --git a/src/commands/bgsave-cancel.json b/src/commands/bgsave-cancel.json index b91b6d8427..66351b246c 100644 --- a/src/commands/bgsave-cancel.json +++ b/src/commands/bgsave-cancel.json @@ -1,6 +1,6 @@ { "CANCEL": { - "summary": "Terminates a running bgsave child process.", + "summary": "Unschedule all scheduled bgsave requests.", "complexity": "O(1)", "group": "server", "since": "8.0.0", diff --git a/src/commands/bgsave-kill.json b/src/commands/bgsave-kill.json new file mode 100644 index 0000000000..111b764022 --- /dev/null +++ b/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" + } + } +} diff --git a/src/commands/bgsave-schedule.json b/src/commands/bgsave-schedule.json new file mode 100644 index 0000000000..556e3798dd --- /dev/null +++ b/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" + } + } +} diff --git a/src/commands/bgsave.json b/src/commands/bgsave.json index f73d8a89b5..2227a0c8d8 100644 --- a/src/commands/bgsave.json +++ b/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" } } } diff --git a/src/rdb.c b/src/rdb.c index 13f3a3e4ea..fc839eadfe 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3636,33 +3636,7 @@ void saveCommand(client *c) { } } -/* BGSAVE CANCEL */ -void bgsaveCancelCommand(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 canceled by user"); - killRDBChild(); - addReply(c, shared.ok); - } else { - addReplyError(c, "Background bgsave is currently not in progress"); - } -} - -/* 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); @@ -3685,6 +3659,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 diff --git a/src/server.h b/src/server.h index d5020dc596..2df7450979 100644 --- a/src/server.h +++ b/src/server.h @@ -3585,6 +3585,8 @@ 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); diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index f1c8129b4a..1a734bc21a 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -171,7 +171,7 @@ start_server {} { assert_equal [s rdb_changes_since_last_save] 0 } - test {bgsave cancel aborts save} { + test {bgsave kill aborts save} { r config set save "" # Generating RDB will take some 100 seconds r config set rdb-key-save-delay 1000000 @@ -185,7 +185,7 @@ start_server {} { } set fork_child_pid [get_child_pid 0] - assert {[r bgsave cancel] eq {OK}} + 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 { @@ -195,7 +195,7 @@ start_server {} { } } - test {bgsave cancel response with error when no ongoing save} { + 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 @@ -206,7 +206,31 @@ start_server {} { } else { fail "bgsave is currently running" } - assert_error "ERR Background bgsave is currently not in progress" {r bgsave cancel} + 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} } From 23bd3a28fef2570c4e49b34072424f8d4b260344 Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Thu, 18 Jul 2024 19:51:28 +0300 Subject: [PATCH 14/32] format fixes Signed-off-by: Ran Shidlansik --- src/rdb.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index fc839eadfe..0d071ab5d6 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3637,7 +3637,6 @@ void saveCommand(client *c) { } void bgsaveCommandInternal(client *c, int schedule) { - rdbSaveInfo rsi, *rsiptr; rsiptr = rdbPopulateSaveInfo(&rsi); From be0954ad9c5c6f6739873604a459f62a024ea52d Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Sat, 20 Jul 2024 12:11:45 +0300 Subject: [PATCH 15/32] remove bgsave kill subcommand and make all logic inside bgsave cancel Signed-off-by: Ran Shidlansik --- src/commands.def | 20 +------------------- src/commands/bgsave-cancel.json | 2 +- src/commands/bgsave-kill.json | 21 --------------------- src/rdb.c | 21 ++++++--------------- src/server.h | 1 - tests/integration/rdb.tcl | 12 ++++++------ 6 files changed, 14 insertions(+), 63 deletions(-) delete mode 100644 src/commands/bgsave-kill.json diff --git a/src/commands.def b/src/commands.def index 96b653859c..2fe2b22110 100644 --- a/src/commands.def +++ b/src/commands.def @@ -6329,23 +6329,6 @@ struct COMMAND_STRUCT ACL_Subcommands[] = { #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 @@ -6365,8 +6348,7 @@ struct COMMAND_STRUCT ACL_Subcommands[] = { /* 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("cancel","Terminates a running bgsave child process. This will also 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("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} }; diff --git a/src/commands/bgsave-cancel.json b/src/commands/bgsave-cancel.json index 66351b246c..4f7e63c8b6 100644 --- a/src/commands/bgsave-cancel.json +++ b/src/commands/bgsave-cancel.json @@ -1,6 +1,6 @@ { "CANCEL": { - "summary": "Unschedule all scheduled bgsave requests.", + "summary": "Terminates a running bgsave child process. This will also unschedule all scheduled bgsave requests.", "complexity": "O(1)", "group": "server", "since": "8.0.0", diff --git a/src/commands/bgsave-kill.json b/src/commands/bgsave-kill.json deleted file mode 100644 index 111b764022..0000000000 --- a/src/commands/bgsave-kill.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "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" - } - } -} diff --git a/src/rdb.c b/src/rdb.c index 0d071ab5d6..57597ec9c5 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3658,29 +3658,20 @@ void bgsaveCommandInternal(client *c, int schedule) { } } -/* BGSAVE KILL */ -void bgsaveKillCommand(client *c) { +/* BGSAVE CANCEL */ +void bgsaveCancelCommand(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"); + serverLog(LL_NOTICE, "Background bgsave will be aborted due to user request"); 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"); + } else if (server.rdb_bgsave_scheduled == 1) { + serverLog(LL_NOTICE, "Scheduled background bgsave will be cancelled due to user request"); server.rdb_bgsave_scheduled = 0; addReply(c, shared.ok); } else { - addReplyError(c, "Background bgsave is currently not scheduled"); + addReplyError(c, "Background bgsave is currently not in progress or scheduled"); } } diff --git a/src/server.h b/src/server.h index 2df7450979..d8cffe91ab 100644 --- a/src/server.h +++ b/src/server.h @@ -3585,7 +3585,6 @@ 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); diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index 1a734bc21a..d4afeeff63 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -171,7 +171,7 @@ start_server {} { assert_equal [s rdb_changes_since_last_save] 0 } - test {bgsave kill aborts save} { + test {bgsave cancel aborts save} { r config set save "" # Generating RDB will take some 100 seconds r config set rdb-key-save-delay 1000000 @@ -185,7 +185,7 @@ start_server {} { } set fork_child_pid [get_child_pid 0] - assert {[r bgsave kill] eq {OK}} + assert {[r bgsave cancel] 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 { @@ -195,7 +195,7 @@ start_server {} { } } - test {bgsave kill response with error when no ongoing save} { + test {bgsave cancel 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 @@ -206,10 +206,10 @@ start_server {} { } else { fail "bgsave is currently running" } - assert_error "ERR Background bgsave is currently not in progress" {r bgsave kill} + assert_error "ERR Background bgsave is currently not in progress or scheduled" {r bgsave cancel} } - test {bgsave cancel request} { + test {bgsave cancel schedulled request} { r config set save "" # Generating RDB will take some 100 seconds r config set rdb-key-save-delay 1000000 @@ -230,7 +230,7 @@ start_server {} { 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} + assert_error "ERR Background bgsave is currently not in progress or scheduled" {r bgsave cancel} } From f662c693d386054c33791df8af82a22be344c4fb Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Sun, 21 Jul 2024 08:29:29 +0300 Subject: [PATCH 16/32] make cancel and schedule command arguments Signed-off-by: Ran Shidlansik --- src/commands.def | 56 +++++++------------------------ src/commands/bgsave-cancel.json | 21 ------------ src/commands/bgsave-schedule.json | 23 ------------- src/commands/bgsave.json | 37 ++++++++++++++++++-- src/rdb.c | 56 +++++++++++++++---------------- src/server.h | 2 -- tests/integration/rdb.tcl | 4 +-- 7 files changed, 78 insertions(+), 121 deletions(-) delete mode 100644 src/commands/bgsave-cancel.json delete mode 100644 src/commands/bgsave-schedule.json diff --git a/src/commands.def b/src/commands.def index 2fe2b22110..228e4b583c 100644 --- a/src/commands.def +++ b/src/commands.def @@ -6312,54 +6312,13 @@ 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 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","Terminates a running bgsave child process. This will also 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("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"}, +{"8.0.0","Added the `CANCEL` option."}, }; #endif @@ -6373,6 +6332,17 @@ commandHistory BGSAVE_History[] = { #define BGSAVE_Keyspecs NULL #endif +/* BGSAVE operation argument table */ +struct COMMAND_ARG BGSAVE_operation_Subargs[] = { +{MAKE_ARG("schedule",ARG_TYPE_PURE_TOKEN,-1,"SCHEDULE",NULL,"3.2.2",CMD_ARG_OPTIONAL,0,NULL)}, +{MAKE_ARG("cancel",ARG_TYPE_PURE_TOKEN,-1,"CANCEL",NULL,"8.0.0",CMD_ARG_OPTIONAL,0,NULL)}, +}; + +/* BGSAVE argument table */ +struct COMMAND_ARG BGSAVE_Args[] = { +{MAKE_ARG("operation",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=BGSAVE_operation_Subargs}, +}; + /********** COMMAND COUNT ********************/ #ifndef SKIP_CMD_HISTORY_TABLE @@ -10936,7 +10906,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,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("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,1),.args=BGSAVE_Args}, {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)}, diff --git a/src/commands/bgsave-cancel.json b/src/commands/bgsave-cancel.json deleted file mode 100644 index 4f7e63c8b6..0000000000 --- a/src/commands/bgsave-cancel.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "CANCEL": { - "summary": "Terminates a running bgsave child process. This will also 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" - } - } -} diff --git a/src/commands/bgsave-schedule.json b/src/commands/bgsave-schedule.json deleted file mode 100644 index 556e3798dd..0000000000 --- a/src/commands/bgsave-schedule.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "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" - } - } -} diff --git a/src/commands/bgsave.json b/src/commands/bgsave.json index 2227a0c8d8..a7a9376308 100644 --- a/src/commands/bgsave.json +++ b/src/commands/bgsave.json @@ -13,7 +13,7 @@ ], [ "8.0.0", - "Refactor SCHEDULE as a subcommand" + "Added the `CANCEL` option." ] ], "command_flags": [ @@ -22,9 +22,42 @@ "NOSCRIPT" ], "arguments": [ + { + "name": "operation", + "type": "oneof", + "arguments": [ + { + "name": "schedule", + "token": "SCHEDULE", + "type": "pure-token", + "optional": true, + "since": "3.2.2" + }, + { + "name": "cancel", + "token": "CANCEL", + "type": "pure-token", + "optional": true, + "since": "8.0.0" + } + ] + } ], "reply_schema": { - "const": "Background saving started" + "oneOf": [ + { + "const": "Background saving started" + }, + { + "const": "Background saving scheduled" + }, + { + "const": "Background bgsave cancelled" + }, + { + "const": "Scheduled background bgsave cancelled" + } + ] } } } diff --git a/src/rdb.c b/src/rdb.c index 57597ec9c5..e04f1e0503 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3636,7 +3636,34 @@ void saveCommand(client *c) { } } -void bgsaveCommandInternal(client *c, int 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 if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "cancel")) { + /* Terminates an in progress BGSAVE */ + if (server.child_type == CHILD_TYPE_RDB) { + /* There is an ongoing save */ + serverLog(LL_NOTICE, "Background bgsave will be aborted due to user request"); + killRDBChild(); + addReplyStatus(c, "Background bgsave cancelled"); + } else if (server.rdb_bgsave_scheduled == 1) { + serverLog(LL_NOTICE, "Scheduled background bgsave will be cancelled due to user request"); + server.rdb_bgsave_scheduled = 0; + addReplyStatus(c, "Scheduled background bgsave cancelled"); + } else { + addReplyError(c, "Background bgsave is currently not in progress or scheduled"); + } + return; + } else { + addReplyErrorObject(c, shared.syntaxerr); + return; + } + } rdbSaveInfo rsi, *rsiptr; rsiptr = rdbPopulateSaveInfo(&rsi); @@ -3658,33 +3685,6 @@ void bgsaveCommandInternal(client *c, int schedule) { } } -/* BGSAVE CANCEL */ -void bgsaveCancelCommand(client *c) { - /* Terminates an in progress BGSAVE */ - if (server.child_type == CHILD_TYPE_RDB) { - /* There is an ongoing save */ - serverLog(LL_NOTICE, "Background bgsave will be aborted due to user request"); - killRDBChild(); - addReply(c, shared.ok); - } else if (server.rdb_bgsave_scheduled == 1) { - serverLog(LL_NOTICE, "Scheduled background bgsave will be cancelled due to user request"); - server.rdb_bgsave_scheduled = 0; - addReply(c, shared.ok); - } else { - addReplyError(c, "Background bgsave is currently not in progress or 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 diff --git a/src/server.h b/src/server.h index d8cffe91ab..73f68a73d4 100644 --- a/src/server.h +++ b/src/server.h @@ -3584,8 +3584,6 @@ void dbsizeCommand(client *c); void lastsaveCommand(client *c); void saveCommand(client *c); void bgsaveCommand(client *c); -void bgsaveCancelCommand(client *c); -void bgsaveScheduleCommand(client *c); void bgrewriteaofCommand(client *c); void shutdownCommand(client *c); void slowlogCommand(client *c); diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index d4afeeff63..c1e9d4abda 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -185,7 +185,7 @@ start_server {} { } set fork_child_pid [get_child_pid 0] - assert {[r bgsave cancel] eq {OK}} + assert {[r bgsave cancel] eq {Background bgsave cancelled}} 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 { @@ -227,7 +227,7 @@ start_server {} { assert {[r bgsave schedule] eq {Background saving scheduled}} # Cancel the scheduled save - assert {[r bgsave cancel] eq {OK}} + assert {[r bgsave cancel] eq {Scheduled background bgsave cancelled}} # Make sure a second call to bgsave cancel return an error assert_error "ERR Background bgsave is currently not in progress or scheduled" {r bgsave cancel} From 7ca24524dffe3d5a1a60ad8814abb15405e59b37 Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Sun, 21 Jul 2024 08:37:16 +0300 Subject: [PATCH 17/32] bring back some extra deleted lines Signed-off-by: Ran Shidlansik --- src/rdb.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rdb.c b/src/rdb.c index e04f1e0503..fb7e254447 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3636,6 +3636,7 @@ void saveCommand(client *c) { } } +/* BGSAVE [SCHEDULE] */ void bgsaveCommand(client *c) { int schedule = 0; @@ -3664,6 +3665,7 @@ void bgsaveCommand(client *c) { return; } } + rdbSaveInfo rsi, *rsiptr; rsiptr = rdbPopulateSaveInfo(&rsi); From 64b52d7887c7834e8668949d8b473739686af23f Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Sun, 21 Jul 2024 08:39:08 +0300 Subject: [PATCH 18/32] format fix Signed-off-by: Ran Shidlansik --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index fb7e254447..d66bfc07b2 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3665,7 +3665,7 @@ void bgsaveCommand(client *c) { return; } } - + rdbSaveInfo rsi, *rsiptr; rsiptr = rdbPopulateSaveInfo(&rsi); From 8ebca1a28a28097278248541c180fd78b3ce124a Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Sun, 21 Jul 2024 11:19:19 +0300 Subject: [PATCH 19/32] 1. marke all bgsave arguments as optional 2. improve bgsave cancel test Signed-off-by: Ran Shidlansik --- src/commands.def | 6 +++--- src/commands/bgsave.json | 3 +-- tests/integration/rdb.tcl | 8 +------- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/commands.def b/src/commands.def index 228e4b583c..0d1da266cd 100644 --- a/src/commands.def +++ b/src/commands.def @@ -6334,13 +6334,13 @@ commandHistory BGSAVE_History[] = { /* BGSAVE operation argument table */ struct COMMAND_ARG BGSAVE_operation_Subargs[] = { -{MAKE_ARG("schedule",ARG_TYPE_PURE_TOKEN,-1,"SCHEDULE",NULL,"3.2.2",CMD_ARG_OPTIONAL,0,NULL)}, -{MAKE_ARG("cancel",ARG_TYPE_PURE_TOKEN,-1,"CANCEL",NULL,"8.0.0",CMD_ARG_OPTIONAL,0,NULL)}, +{MAKE_ARG("schedule",ARG_TYPE_PURE_TOKEN,-1,"SCHEDULE",NULL,"3.2.2",CMD_ARG_NONE,0,NULL)}, +{MAKE_ARG("cancel",ARG_TYPE_PURE_TOKEN,-1,"CANCEL",NULL,"8.0.0",CMD_ARG_NONE,0,NULL)}, }; /* BGSAVE argument table */ struct COMMAND_ARG BGSAVE_Args[] = { -{MAKE_ARG("operation",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=BGSAVE_operation_Subargs}, +{MAKE_ARG("operation",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,2,NULL),.subargs=BGSAVE_operation_Subargs}, }; /********** COMMAND COUNT ********************/ diff --git a/src/commands/bgsave.json b/src/commands/bgsave.json index a7a9376308..16bfe673b2 100644 --- a/src/commands/bgsave.json +++ b/src/commands/bgsave.json @@ -25,19 +25,18 @@ { "name": "operation", "type": "oneof", + "optional": true, "arguments": [ { "name": "schedule", "token": "SCHEDULE", "type": "pure-token", - "optional": true, "since": "3.2.2" }, { "name": "cancel", "token": "CANCEL", "type": "pure-token", - "optional": true, "since": "8.0.0" } ] diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index c1e9d4abda..0356c4da8d 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -193,14 +193,8 @@ start_server {} { } else { fail "bgsave temp file was not deleted after cancel" } - } - - test {bgsave cancel 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 + # Make sure no save is running and that bgsave return an error wait_for_condition 50 100 { [s rdb_bgsave_in_progress] == 0 } else { From 9873d640efdd15e41e1a139f796566f202cbb3df Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:36:35 +0300 Subject: [PATCH 20/32] Update src/rdb.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Viktor Söderqvist Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index d66bfc07b2..6d0dca2713 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3651,7 +3651,7 @@ void bgsaveCommand(client *c) { /* There is an ongoing save */ serverLog(LL_NOTICE, "Background bgsave will be aborted due to user request"); killRDBChild(); - addReplyStatus(c, "Background bgsave cancelled"); + addReplyStatus(c, "Background saving cancelled"); } else if (server.rdb_bgsave_scheduled == 1) { serverLog(LL_NOTICE, "Scheduled background bgsave will be cancelled due to user request"); server.rdb_bgsave_scheduled = 0; From c1b6be771068c418b3542ff22a69fd72521f9426 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:36:45 +0300 Subject: [PATCH 21/32] Update src/rdb.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Viktor Söderqvist Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index 6d0dca2713..9c7d60039a 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3655,7 +3655,7 @@ void bgsaveCommand(client *c) { } else if (server.rdb_bgsave_scheduled == 1) { serverLog(LL_NOTICE, "Scheduled background bgsave will be cancelled due to user request"); server.rdb_bgsave_scheduled = 0; - addReplyStatus(c, "Scheduled background bgsave cancelled"); + addReplyStatus(c, "Scheduled background saving cancelled"); } else { addReplyError(c, "Background bgsave is currently not in progress or scheduled"); } From e80173b94147e2a6180e0efc3564f84703b241a2 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:37:06 +0300 Subject: [PATCH 22/32] Update src/rdb.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Viktor Söderqvist Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index 9c7d60039a..93dd626d57 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3657,7 +3657,7 @@ void bgsaveCommand(client *c) { server.rdb_bgsave_scheduled = 0; addReplyStatus(c, "Scheduled background saving cancelled"); } else { - addReplyError(c, "Background bgsave is currently not in progress or scheduled"); + addReplyError(c, "Background saving is currently not in progress or scheduled"); } return; } else { From c2c09d3cd07c713b35fc4ed1e0eb008d975745c9 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:37:24 +0300 Subject: [PATCH 23/32] Update src/commands/bgsave.json Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/commands/bgsave.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/bgsave.json b/src/commands/bgsave.json index 16bfe673b2..7429011042 100644 --- a/src/commands/bgsave.json +++ b/src/commands/bgsave.json @@ -51,7 +51,7 @@ "const": "Background saving scheduled" }, { - "const": "Background bgsave cancelled" + "const": "Background saving cancelled" }, { "const": "Scheduled background bgsave cancelled" From a8a2c544c43dd2d192d3c339d44b9abafffaa7cf Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:37:32 +0300 Subject: [PATCH 24/32] Update src/commands/bgsave.json Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/commands/bgsave.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/bgsave.json b/src/commands/bgsave.json index 7429011042..cf1c920474 100644 --- a/src/commands/bgsave.json +++ b/src/commands/bgsave.json @@ -54,7 +54,7 @@ "const": "Background saving cancelled" }, { - "const": "Scheduled background bgsave cancelled" + "const": "Scheduled background saving cancelled" } ] } From 445ebe0c27e290d46ca6bb14f5c060adfd480df1 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:37:45 +0300 Subject: [PATCH 25/32] Update src/rdb.c Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index 93dd626d57..92599c2bce 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3649,7 +3649,7 @@ void bgsaveCommand(client *c) { /* Terminates an in progress BGSAVE */ if (server.child_type == CHILD_TYPE_RDB) { /* There is an ongoing save */ - serverLog(LL_NOTICE, "Background bgsave will be aborted due to user request"); + serverLog(LL_NOTICE, "Background saving will be aborted due to user request"); killRDBChild(); addReplyStatus(c, "Background saving cancelled"); } else if (server.rdb_bgsave_scheduled == 1) { From 7ae21bbb35881fe8efa68ca72d6aa84a49a97dc2 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:37:57 +0300 Subject: [PATCH 26/32] Update tests/integration/rdb.tcl Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- tests/integration/rdb.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index 0356c4da8d..670828d7ea 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -185,7 +185,7 @@ start_server {} { } set fork_child_pid [get_child_pid 0] - assert {[r bgsave cancel] eq {Background bgsave cancelled}} + assert {[r bgsave cancel] eq {Background saving cancelled}} 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 { From ae9fff6e3a5e58b5a1505d90f91bb7ba5d3cfc4c Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:38:04 +0300 Subject: [PATCH 27/32] Update tests/integration/rdb.tcl Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- tests/integration/rdb.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index 670828d7ea..d183680ec9 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -221,7 +221,7 @@ start_server {} { assert {[r bgsave schedule] eq {Background saving scheduled}} # Cancel the scheduled save - assert {[r bgsave cancel] eq {Scheduled background bgsave cancelled}} + assert {[r bgsave cancel] eq {Scheduled background saving cancelled}} # Make sure a second call to bgsave cancel return an error assert_error "ERR Background bgsave is currently not in progress or scheduled" {r bgsave cancel} From db9353e9bc5d0ceb0ab4c53e68af420087099a33 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:38:11 +0300 Subject: [PATCH 28/32] Update tests/integration/rdb.tcl Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- tests/integration/rdb.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index d183680ec9..c6c4c17f70 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -224,7 +224,7 @@ start_server {} { assert {[r bgsave cancel] eq {Scheduled background saving cancelled}} # Make sure a second call to bgsave cancel return an error - assert_error "ERR Background bgsave is currently not in progress or scheduled" {r bgsave cancel} + assert_error "ERR Background saving is currently not in progress or scheduled" {r bgsave cancel} } From 6c0c85506311c863eccd8439bcb3316511f30999 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:38:22 +0300 Subject: [PATCH 29/32] Update tests/integration/rdb.tcl Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- tests/integration/rdb.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index c6c4c17f70..ea5ed82e32 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -200,7 +200,7 @@ start_server {} { } else { fail "bgsave is currently running" } - assert_error "ERR Background bgsave is currently not in progress or scheduled" {r bgsave cancel} + assert_error "ERR Background saving is currently not in progress or scheduled" {r bgsave cancel} } test {bgsave cancel schedulled request} { From 057625438788697bb8902b72314323b5a4ab79f4 Mon Sep 17 00:00:00 2001 From: ranshid <88133677+ranshid@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:38:32 +0300 Subject: [PATCH 30/32] Update src/rdb.c Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com> --- src/rdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index 92599c2bce..cb51ceefa1 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3653,7 +3653,7 @@ void bgsaveCommand(client *c) { killRDBChild(); addReplyStatus(c, "Background saving cancelled"); } else if (server.rdb_bgsave_scheduled == 1) { - serverLog(LL_NOTICE, "Scheduled background bgsave will be cancelled due to user request"); + serverLog(LL_NOTICE, "Scheduled background saving will be cancelled due to user request"); server.rdb_bgsave_scheduled = 0; addReplyStatus(c, "Scheduled background saving cancelled"); } else { From 71533deace0df8bef5a48398af38e5c9a51a86ad Mon Sep 17 00:00:00 2001 From: Binbin Date: Wed, 24 Jul 2024 10:34:29 +0800 Subject: [PATCH 31/32] Apply suggestions from code review Signed-off-by: Binbin --- src/rdb.c | 2 +- tests/integration/rdb.tcl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index cb51ceefa1..b9b4682db9 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3648,7 +3648,7 @@ void bgsaveCommand(client *c) { } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "cancel")) { /* Terminates an in progress BGSAVE */ if (server.child_type == CHILD_TYPE_RDB) { - /* There is an ongoing save */ + /* There is an ongoing bgsave */ serverLog(LL_NOTICE, "Background saving will be aborted due to user request"); killRDBChild(); addReplyStatus(c, "Background saving cancelled"); diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index ea5ed82e32..61cb0cea7e 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -181,7 +181,7 @@ start_server {} { wait_for_condition 50 100 { [s rdb_bgsave_in_progress] == 1 } else { - fail "bgsave not done" + fail "bgsave did not start in time" } set fork_child_pid [get_child_pid 0] From 55a1541b1674a424a077c5cf70281e9961e2a691 Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Sun, 28 Jul 2024 08:47:20 +0300 Subject: [PATCH 32/32] add log msg for bgsave schedule Signed-off-by: Ran Shidlansik --- src/rdb.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/rdb.c b/src/rdb.c index b9b4682db9..3fcde978dd 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3674,6 +3674,11 @@ void bgsaveCommand(client *c) { } else if (hasActiveChildProcess() || server.in_exec) { if (schedule || server.in_exec) { server.rdb_bgsave_scheduled = 1; + if (schedule) { + serverLog(LL_NOTICE, "Background saving scheduled due to user request"); + } else { + serverLog(LL_NOTICE, "Background saving scheduled to run after transaction execution"); + } addReplyStatus(c, "Background saving scheduled"); } else { addReplyError(c, "Another child process is active (AOF?): can't BGSAVE right now. "