From cb1717865804fdb0561e728d2f3a0a1138099d9d Mon Sep 17 00:00:00 2001 From: Binbin Date: Wed, 29 Mar 2023 20:17:05 +0800 Subject: Fix fork done handler wrongly update fsync metrics and enhance AOF_ FSYNC_ALWAYS (#11973) This PR fix several unrelated bugs that were discovered by the same set of tests (WAITAOF tests in #11713), could make the `WAITAOF` test hang. The change in `backgroundRewriteDoneHandler` is about MP-AOF. That leftover / old code assumes that we started a new AOF file just now (when we have a new base into which we're gonna incrementally write), but the fact is that with MP-AOF, the fork done handler doesn't really affect the incremental file being maintained by the parent process, there's no reason to re-issue `SELECT`, and no reason to update any of the fsync variables in that flow. This should have been deleted with MP-AOF (introduced in #9788, 7.0). The damage is that the update to `aof_fsync_offset` will cause us to miss an fsync in `flushAppendOnlyFile`, that happens if we stop write commands in `AOF_FSYNC_EVERYSEC` while an AOFRW is in progress. This caused a new `WAITAOF` test to sometime hang forever. Also because of MP-AOF, we needed to change `aof_fsync_offset` to `aof_last_incr_fsync_offset` and match it to `aof_last_incr_size` in `flushAppendOnlyFile`. This is because in the past we compared `aof_fsync_offset` and `aof_current_size`, but with MP-AOF it could be the total AOF file will be smaller after AOFRW, and the (already existing) incr file still has data that needs to be fsynced. The change in `flushAppendOnlyFile`, about the `AOF_FSYNC_ALWAYS`, it is follow #6053 (the details is in #5985), we also check `AOF_FSYNC_ALWAYS` to handle a case where appendfsync is changed from everysec to always while there is data that's written but not yet fsynced. --- src/server.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/server.h') diff --git a/src/server.h b/src/server.h index 1a68071f1..43d85008a 100644 --- a/src/server.h +++ b/src/server.h @@ -1742,7 +1742,8 @@ struct redisServer { off_t aof_rewrite_base_size; /* AOF size on latest startup or rewrite. */ off_t aof_current_size; /* AOF current size (Including BASE + INCRs). */ off_t aof_last_incr_size; /* The size of the latest incr AOF. */ - off_t aof_fsync_offset; /* AOF offset which is already synced to disk. */ + off_t aof_last_incr_fsync_offset; /* AOF offset which is already requested to be synced to disk. + * Compare with the aof_last_incr_size. */ int aof_flush_sleep; /* Micros to sleep before flush. (used by tests) */ int aof_rewrite_scheduled; /* Rewrite once BGSAVE terminates. */ sds aof_buf; /* AOF buffer, written before entering the event loop */ -- cgit v1.2.1