• Binbin's avatar
    Fix fork done handler wrongly update fsync metrics and enhance AOF_ FSYNC_ALWAYS (#11973) · cb171786
    Binbin authored
    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.
    cb171786
aof.c 102 KB