Change Details

This fixes a possible (unlikely) data race when commiting block files. rationale from [[https://github.com/bitcoin/bitcoin/pull/12696 | core#12696]] > It was recently pointed out to me that calling fsync() or fdatasync() on a new file is not sufficient to ensure it's persisted to disk, as the existence of the file itself is stored in the directory inode. This means that ensuring that a new file is actually committed also requires an fsync() on the parent directory. This only applies to new files, calling fsync() on an old file is always fine. > > In theory this means that the block index can race block persistence, as a poorly timed power outage could cause us to commit data to the block index that gets lost in the filesystem. The change here ensures that we call fsync() on the blocks directory after committing new block files. I checked the LevelDB source code and they already do this when updating their writeahead log. In theory this could happen at the same time as a chain split and that could cause you to come back online and then miss the block you had committed to the index, which would put you permanently out of sync between the two. This seems pretty far fetched, but we should handle this case correctly anyway. Prefer Mac-specific F_FULLSYNC over fdatasync in FileCommit, as MacOS can erroneously report that it has fdatasync. See discussion here: https://github.com/haskell/unix/issues/37 Define and use HAVE_FDATASYNC correctly. Previously it was never defined and `FileCommit` always used `fsync`. This was not a big problem, it just caused a small performance loss, but if we want to maintain this code path it is better if it actually works. This is a backport of [[https://github.com/bitcoin/bitcoin/pull/14501 | core#14501]] Depends on D16151

This fixes a possible (unlikely) data race when commiting block files. rationale from [[https://github.com/bitcoin/bitcoin/pull/12696 | core#12696]] > It was recently pointed out to me that calling fsync() or fdatasync() on a new file is not sufficient to ensure it's persisted to disk, as the existence of the file itself is stored in the directory inode. This means that ensuring that a new file is actually committed also requires an fsync() on the parent directory. This only applies to new files, calling fsync() on an old file is always fine. > > In theory this means that the block index can race block persistence, as a poorly timed power outage could cause us to commit data to the block index that gets lost in the filesystem. The change here ensures that we call fsync() on the blocks directory after committing new block files. I checked the LevelDB source code and they already do this when updating their writeahead log. In theory this could happen at the same time as a chain split and that could cause you to come back online and then miss the block you had committed to the index, which would put you permanently out of sync between the two. This seems pretty far fetched, but we should handle this case correctly anyway. Prefer Mac-specific F_FULLSYNC over fdatasync in FileCommit, as MacOS can erroneously report that it has fdatasync. See discussion here: https://github.com/haskell/unix/issues/37 This was not an issue before this diff, because HAVE_FDATASYNC was never properly defined (see next point below). Define and use HAVE_FDATASYNC correctly. Previously it was never defined and `FileCommit` always used `fsync`. This was not a big problem, it just caused a small performance loss, but if we want to maintain this code path it is better if it actually works. This is a backport of [[https://github.com/bitcoin/bitcoin/pull/14501 | core#14501]] Depends on D16151

This fixes a possible (unlikely) data race when commiting block files. rationale from [[https://github.com/bitcoin/bitcoin/pull/12696 | core#12696]] > It was recently pointed out to me that calling fsync() or fdatasync() on a new file is not sufficient to ensure it's persisted to disk, as the existence of the file itself is stored in the directory inode. This means that ensuring that a new file is actually committed also requires an fsync() on the parent directory. This only applies to new files, calling fsync() on an old file is always fine. > > In theory this means that the block index can race block persistence, as a poorly timed power outage could cause us to commit data to the block index that gets lost in the filesystem. The change here ensures that we call fsync() on the blocks directory after committing new block files. I checked the LevelDB source code and they already do this when updating their writeahead log. In theory this could happen at the same time as a chain split and that could cause you to come back online and then miss the block you had committed to the index, which would put you permanently out of sync between the two. This seems pretty far fetched, but we should handle this case correctly anyway. Prefer Mac-specific F_FULLSYNC over fdatasync in FileCommit, as MacOS can erroneously report that it has fdatasync. See discussion here: https://github.com/haskell/unix/issues/37 This was not an issue before this diff, because HAVE_FDATASYNC was never properly defined (see next point below). Define and use HAVE_FDATASYNC correctly. Previously it was never defined and `FileCommit` always used `fsync`. This was not a big problem, it just caused a small performance loss, but if we want to maintain this code path it is better if it actually works. This is a backport of [[https://github.com/bitcoin/bitcoin/pull/14501 | core#14501]] Depends on D16151