Skip to content

aof: enhance AOF_FSYNC_EVERYSEC, more details in #5985#6053

Merged
antirez merged 1 commit intoredis:unstablefrom
soloestoy:enhance-aof-fsync-everysec
May 10, 2019
Merged

aof: enhance AOF_FSYNC_EVERYSEC, more details in #5985#6053
antirez merged 1 commit intoredis:unstablefrom
soloestoy:enhance-aof-fsync-everysec

Conversation

@soloestoy
Copy link
Contributor

No description provided.

@soloestoy soloestoy force-pushed the enhance-aof-fsync-everysec branch from c68c9eb to bcac165 Compare April 29, 2019 06:39
@antirez antirez merged commit e633254 into redis:unstable May 10, 2019
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Jul 10, 2019
aof: enhance AOF_FSYNC_EVERYSEC, more details in redis#5985
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Mar 26, 2023
…YNC_ALWAYS

This PR fix two bugs:

1. About the MP-AOF one, that 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 multipart-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 i suppose there's no reason to update any of the fsync metrics.
This should have been deleted with MP-AOF (introduced in redis#9788, 7.0).
The aof_fsync_offset it updates will cause us to miss an fsync in flushAppendOnlyFile,
if we stop write commands in AOF_FSYNC_EVERYSEC

2. About the AOF_FSYNC_ALWAYS one, it is follow redis#6053 (the details is in redis#5985),
we also check AOF_FSYNC_ALWAYS to prevent users from switching from everysec to always.

We detected these problems with WAITAOF test (redis 7.2), introduced in redis#11713.
oranagra pushed a commit that referenced this pull request Mar 29, 2023
…SYNC_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.
oranagra pushed a commit that referenced this pull request Apr 17, 2023
…SYNC_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.

(cherry picked from commit cb17178)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants