Add F_BARRIERFSYNC for Sync operations on MacOS #319
Conversation
Allow the posix sync code to be based upon BARRIER sync
|
Here are the test times via cmake and ctest: AUTO: ctest -j 8 3636.12s user 1284.08s system 154% cpu 53:01.82 total |
| @@ -102,6 +102,47 @@ int Madvise(void* addr, size_t len, int advice) { | |||
| } | |||
|
|
|||
| namespace { | |||
| IOStatus PosixSync(int fd, const std::string& file_name, | |||
| const char* file_type) { | |||
| #if defined(HAVE_BARRIERFSYNC) | |||
There was a problem hiding this comment.
this is code is the same as in the PosixFSync. do a function PosixSync that as parameter you pass if it is fsync or not and then split to the different code
There was a problem hiding this comment.
I thought about making one routine for sync/fsync, passing in an argument. But the code is NOT the same:
- In the non-Mac case, Sync uses fdatasync and FSync uses fsync
- In the Mac case, Sync prefers BARRIER and FULL prefers FSync but falls back to the other
I thought about merging them, but the code actually ends up more complicated. If you have a code layout that is simpler, I will take it but I could not come up with one.
There was a problem hiding this comment.
forget. its not worth the complications. go ahead
|
|
||
| IOStatus PosixFSync(int fd, const std::string& file_name, | ||
| const char* file_type) { | ||
| #if defined(HAVE_FULLFSYNC) |
| #else // HAVE_FULLFSYNC | ||
| if (fdatasync(fd_) < 0) { | ||
| return IOError("While fdatasync mmapped file", filename_, errno); | ||
| IOStatus s = PosixSync(fd_, filename_, "mmapped file"); |
There was a problem hiding this comment.
need to call generic PosixSync with fsync false
| #else // HAVE_FULLFSYNC | ||
| if (fsync(fd_) < 0) { | ||
| return IOError("While fsync mmaped file", filename_, errno); | ||
| auto s = PosixFSync(fd_, filename_, "mmapped file"); |
There was a problem hiding this comment.
need to call generic PosixSync with fsync true
| assert(!is_btrfs_); | ||
| if (fd_ != -1 && ::fcntl(fd_, F_FULLFSYNC) < 0) { | ||
| return IOError("while fcntl(F_FULLFSYNC)", "a directory", errno); | ||
| if (fd_ != -1) { |
|
@Yuval-Ariel - please run QA. |
|
@Guyme , the QA per issue is done using the CI.. once the reviewer approves, the QA runs automatically |
|
fuzzer passed manually |
|
@mrambacher has this been to performance? |
|
on hold until commit msg is fixed |
Allow the posix sync code to be based upon BARRIER sync
Allow the posix sync code to be based upon BARRIER sync
Allow the posix sync code to be based upon BARRIER sync
Allow the posix sync code to be based upon BARRIER sync
Allow the posix sync code to be based upon BARRIER sync
Allow the posix sync code to be based upon BARRIER sync
This code is an attempt to address facebook/rocksdb#11035.
On MacOS, fsync does not have the same durability guarantees as under Linux. To address this, a f_cntl(F_FULLFSYNC) call was used added in this RocksDB PR. This original PR made every Sync and FSync operation use F_FULLFSYNC if it was available, at a great performance expense.
Apple also has an option "F_BARRIERFSYNC" which it says should be sufficient for most applications. Other useful discussions on full or barrier sync can be found here or here.
This PR introduces a new Make/CMake option FSYNC_MODE. This option does the following:
The default is AUTO.
On my Mac, I see the following performance from timed make check runs:
FSYNC_MODE=AUTO make check 2787.87s user 2153.54s system 78% cpu 1:44:32.69 total
FSYNC_MODE=OFF make check 3081.92s user 898.34s system 276% cpu 24:00.67 total
FSYNC_MODE=BARRIER make check 3423.41s user 1437.35s system 160% cpu 50:21.54 total
FSYNC_MODE=FULL make check 4329.64s user 1909.05s system 63% cpu 2:43:40.98 total
Tests for CMake will be forthcoming.