Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce bgsave cancel #757

Merged
merged 32 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
bba9bfc
Introduce bgsave cancel
ranshid Jul 8, 2024
09ac835
clang format fix
ranshid Jul 8, 2024
7899fad
Update src/rdb.c
ranshid Jul 9, 2024
0572c7a
Update src/rdb.c
ranshid Jul 9, 2024
c25a85b
Update tests/integration/rdb.tcl
ranshid Jul 9, 2024
9f7f3aa
Update tests/integration/rdb.tcl
ranshid Jul 9, 2024
50bc3f0
change bgsave cancel to be a sub-command
ranshid Jul 9, 2024
8aff7ac
fix bad merge-stash
ranshid Jul 9, 2024
c7b71ec
Update src/rdb.c
ranshid Jul 9, 2024
6b823e3
some minor extra fixes
ranshid Jul 9, 2024
63f9529
fix rdb.tcl after error string change
ranshid Jul 9, 2024
1878f86
Update src/rdb.c
ranshid Jul 10, 2024
528038f
1. change bgsave schedule to be a sub-command
ranshid Jul 18, 2024
23bd3a2
format fixes
ranshid Jul 18, 2024
be0954a
remove bgsave kill subcommand and make all logic inside bgsave cancel
ranshid Jul 20, 2024
f662c69
make cancel and schedule command arguments
ranshid Jul 21, 2024
7ca2452
bring back some extra deleted lines
ranshid Jul 21, 2024
64b52d7
format fix
ranshid Jul 21, 2024
8ebca1a
1. marke all bgsave arguments as optional
ranshid Jul 21, 2024
9873d64
Update src/rdb.c
ranshid Jul 22, 2024
c1b6be7
Update src/rdb.c
ranshid Jul 22, 2024
e80173b
Update src/rdb.c
ranshid Jul 22, 2024
c2c09d3
Update src/commands/bgsave.json
ranshid Jul 22, 2024
a8a2c54
Update src/commands/bgsave.json
ranshid Jul 22, 2024
445ebe0
Update src/rdb.c
ranshid Jul 22, 2024
7ae21bb
Update tests/integration/rdb.tcl
ranshid Jul 22, 2024
ae9fff6
Update tests/integration/rdb.tcl
ranshid Jul 22, 2024
db9353e
Update tests/integration/rdb.tcl
ranshid Jul 22, 2024
6c0c855
Update tests/integration/rdb.tcl
ranshid Jul 22, 2024
0576254
Update src/rdb.c
ranshid Jul 22, 2024
71533de
Apply suggestions from code review
enjoy-binbin Jul 24, 2024
55a1541
add log msg for bgsave schedule
ranshid Jul 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/commands.def
Original file line number Diff line number Diff line change
Expand Up @@ -6318,6 +6318,7 @@ struct COMMAND_STRUCT ACL_Subcommands[] = {
/* BGSAVE history */
commandHistory BGSAVE_History[] = {
{"3.2.2","Added the `SCHEDULE` option."},
{"8.0.0","Added the `CANCEL` option."},
};
#endif

Expand All @@ -6331,9 +6332,15 @@ 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_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("schedule",ARG_TYPE_PURE_TOKEN,-1,"SCHEDULE",NULL,"3.2.2",CMD_ARG_OPTIONAL,0,NULL)},
{MAKE_ARG("operation",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,2,NULL),.subargs=BGSAVE_operation_Subargs},
};

/********** COMMAND COUNT ********************/
Expand Down Expand Up @@ -10899,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,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,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)},
Expand Down
30 changes: 26 additions & 4 deletions src/commands/bgsave.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
[
"3.2.2",
"Added the `SCHEDULE` option."
],
[
"8.0.0",
enjoy-binbin marked this conversation as resolved.
Show resolved Hide resolved
"Added the `CANCEL` option."
]
],
"command_flags": [
Expand All @@ -19,11 +23,23 @@
],
"arguments": [
{
"name": "schedule",
"token": "SCHEDULE",
"type": "pure-token",
"name": "operation",
"type": "oneof",
"optional": true,
"since": "3.2.2"
"arguments": [
{
"name": "schedule",
"token": "SCHEDULE",
"type": "pure-token",
"since": "3.2.2"
},
{
"name": "cancel",
"token": "CANCEL",
"type": "pure-token",
"since": "8.0.0"
}
]
}
],
"reply_schema": {
Expand All @@ -33,6 +49,12 @@
},
{
"const": "Background saving scheduled"
},
{
"const": "Background saving cancelled"
},
{
"const": "Scheduled background saving cancelled"
}
]
}
Expand Down
20 changes: 20 additions & 0 deletions src/rdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -3645,6 +3645,21 @@ 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")) {
ranshid marked this conversation as resolved.
Show resolved Hide resolved
/* Terminates an in progress BGSAVE */
if (server.child_type == CHILD_TYPE_RDB) {
/* There is an ongoing bgsave */
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) {
serverLog(LL_NOTICE, "Scheduled background saving will be cancelled due to user request");
ranshid marked this conversation as resolved.
Show resolved Hide resolved
server.rdb_bgsave_scheduled = 0;
addReplyStatus(c, "Scheduled background saving cancelled");
} else {
addReplyError(c, "Background saving is currently not in progress or scheduled");
}
return;
} else {
addReplyErrorObject(c, shared.syntaxerr);
return;
Expand All @@ -3659,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");
}
enjoy-binbin marked this conversation as resolved.
Show resolved Hide resolved
addReplyStatus(c, "Background saving scheduled");
} else {
addReplyError(c, "Another child process is active (AOF?): can't BGSAVE right now. "
Expand Down
58 changes: 58 additions & 0 deletions tests/integration/rdb.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,64 @@ 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 did not start in time"
}
set fork_child_pid [get_child_pid 0]

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 {
![file exists $temp_rdb]
} else {
fail "bgsave temp file was not deleted after cancel"
}
ranshid marked this conversation as resolved.
Show resolved Hide resolved

# 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 saving is currently not in progress or scheduled" {r bgsave cancel}
}

test {bgsave cancel schedulled 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 {Scheduled background saving cancelled}}

# Make sure a second call to bgsave cancel return an error
assert_error "ERR Background saving is currently not in progress or scheduled" {r bgsave cancel}
}


}

test {client freed during loading} {
Expand Down
Loading