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.
```
This commit is contained in:
guybe7 2023-09-28 16:05:53 +02:00 committed by GitHub
parent f924bebd83
commit bfa3931a04
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 47 additions and 12 deletions

View File

@ -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];

View File

@ -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