Commit ba113c83 authored by guybe7's avatar guybe7 Committed by Oran Agra
Browse files

WAITAOF: Update fsynced_reploff_pending just before starting the initial AOFRW fork (#12620)

If we set `fsynced_reploff_pending` in `startAppendOnly`, and the fork doesn't start
immediately (e.g. there's another fork active at the time), any subsequent commands
will increment `server.master_repl_offset`, but will not cause a fsync (given they were
executed before the fork started, they just ended up in the RDB part of it)
Therefore, any WAITAOF will wait on the new master_repl_offset, but it will time out
because no fsync will be executed.

Release notes:
```
WAITAOF could timeout in the absence of write traffic in case a new AOF is created and
an AOFRW can't immediately start.
This can happen by the appendonly config is changed at runtime, but also after FLUSHALL,
and replica full sync.
```

(cherry picked from commit bfa3931a)
parent 348eda81
......@@ -976,18 +976,6 @@ void stopAppendOnly(void) {
int startAppendOnly(void) {
serverAssert(server.aof_state == AOF_OFF);
/* Wait for all bio jobs related to AOF to drain. This prevents a race
* between updates to `fsynced_reploff_pending` of the worker thread, belonging
* to the previous AOF, and the new one. This concern is specific for a full
* sync scenario where we don't wanna risk the ACKed replication offset
* jumping backwards or forward when switching to a different master. */
bioDrainWorker(BIO_AOF_FSYNC);
/* Set the initial repl_offset, which will be applied to fsynced_reploff
* when AOFRW finishes (after possibly being updated by a bio thread) */
atomicSet(server.fsynced_reploff_pending, server.master_repl_offset);
server.fsynced_reploff = 0;
server.aof_state = AOF_WAIT_REWRITE;
if (hasActiveChildProcess() && server.child_type != CHILD_TYPE_AOF) {
server.aof_rewrite_scheduled = 1;
......@@ -2454,7 +2442,23 @@ int rewriteAppendOnlyFileBackground(void) {
server.aof_lastbgrewrite_status = C_ERR;
return C_ERR;
}
if (server.aof_state == AOF_WAIT_REWRITE) {
/* Wait for all bio jobs related to AOF to drain. This prevents a race
* between updates to `fsynced_reploff_pending` of the worker thread, belonging
* to the previous AOF, and the new one. This concern is specific for a full
* sync scenario where we don't wanna risk the ACKed replication offset
* jumping backwards or forward when switching to a different master. */
bioDrainWorker(BIO_AOF_FSYNC);
/* Set the initial repl_offset, which will be applied to fsynced_reploff
* when AOFRW finishes (after possibly being updated by a bio thread) */
atomicSet(server.fsynced_reploff_pending, server.master_repl_offset);
server.fsynced_reploff = 0;
}
server.stat_aof_rewrites++;
if ((childpid = redisFork(CHILD_TYPE_AOF)) == 0) {
char tmpfile[256];
......
......@@ -140,6 +140,37 @@ tags {"wait aof network external:skip"} {
assert_error {ERR WAITAOF cannot be used when numlocal is set but appendonly is disabled.} {$master waitaof 1 0 0}
}
test {WAITAOF local if AOFRW was postponed} {
r config set appendfsync everysec
# turn off AOF
r config set appendonly no
# create an RDB child that takes a lot of time to run
r set x y
r config set rdb-key-save-delay 100000000 ;# 100 seconds
r bgsave
assert_equal [s rdb_bgsave_in_progress] 1
# turn on AOF
r config set appendonly yes
assert_equal [s aof_rewrite_scheduled] 1
# create a write command (to increment master_repl_offset)
r set x y
# reset save_delay and kill RDB child
r config set rdb-key-save-delay 0
catch {exec kill -9 [get_child_pid 0]}
# wait for AOF (will unblock after AOFRW finishes)
assert_equal [r waitaof 1 0 10000] {1 0}
# make sure AOFRW finished
assert_equal [s aof_rewrite_in_progress] 0
assert_equal [s aof_rewrite_scheduled] 0
}
$master config set appendonly yes
waitForBgrewriteaof $master
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment