Skip to content

Commit

Permalink
Fix unexpected behavior when turning appendonly on and off within a t…
Browse files Browse the repository at this point in the history
…ransaction

If we do `config set appendonly yes` and `config set appendonly no`
in a multi, there are some unexpected behavior.

When doing appendonly yes, we will schedule a AOFRW, and when we
are doding appendonly no, we will call stopAppendOnly to stop it.
In stopAppendOnly, the aof_fd is -1 since the aof is not start yet
and the fsync and close will take the -1 and call it.

Signed-off-by: Binbin <[email protected]>
  • Loading branch information
enjoy-binbin committed Jul 24, 2024
1 parent 3cca268 commit 8454509
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 7 deletions.
20 changes: 13 additions & 7 deletions src/aof.c
Original file line number Diff line number Diff line change
Expand Up @@ -931,18 +931,23 @@ void killAppendOnlyChild(void) {
* at runtime using the CONFIG command. */
void stopAppendOnly(void) {
serverAssert(server.aof_state != AOF_OFF);
flushAppendOnlyFile(1);
if (valkey_fsync(server.aof_fd) == -1) {
serverLog(LL_WARNING, "Fail to fsync the AOF file: %s", strerror(errno));
} else {
server.aof_last_fsync = server.mstime;
if (server.aof_fd != -1) {
flushAppendOnlyFile(1);
if (valkey_fsync(server.aof_fd) == -1) {
serverLog(LL_WARNING, "Fail to fsync the AOF file: %s", strerror(errno));
} else {
server.aof_last_fsync = server.mstime;
}
close(server.aof_fd);
}
close(server.aof_fd);

server.aof_fd = -1;
server.aof_selected_db = -1;
server.aof_state = AOF_OFF;
server.aof_rewrite_scheduled = 0;
if (server.aof_rewrite_scheduled) {
server.aof_rewrite_scheduled = 0;
serverLog(LL_NOTICE, "AOF was disabled but there is a scheduled AOF background, cancel it.");
}
server.aof_last_incr_size = 0;
server.aof_last_incr_fsync_offset = 0;
server.fsynced_reploff = -1;
Expand Down Expand Up @@ -2453,6 +2458,7 @@ void bgrewriteaofCommand(client *c) {
/* When manually triggering AOFRW we reset the count
* so that it can be executed immediately. */
server.stat_aofrw_consecutive_failures = 0;
serverLog(LL_NOTICE, "Background append only file rewriting scheduled.");
addReplyStatus(c, "Background append only file rewriting scheduled");
} else if (rewriteAppendOnlyFileBackground() == C_OK) {
addReplyStatus(c, "Background append only file rewriting started");
Expand Down
16 changes: 16 additions & 0 deletions tests/integration/aof.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -655,4 +655,20 @@ tags {"aof external:skip"} {
}
}
}

start_server {} {
test {FLUSHDB / FLUSHALL should persist in AOF} {
r config set appendonly no
r multi
r config set appendonly yes
r config set appendonly no
r exec

r config set appendonly yes
r multi
r config set appendonly no
r config set appendonly yes
r exec
}
}
}

0 comments on commit 8454509

Please sign in to comment.