Use file.Sync() to flush data on linux platform
#615
Conversation
Eventually file.Sync() calls syscall syscall.Fsync, which flushes both data and metadata. While syscall.Fdatasync only flushes data. So file.Sync is safer and it's more readable. Signed-off-by: Benjamin Wang <[email protected]>
ahrtr
changed the title
file.Sync() to flush data on linux platform
| ) | ||
|
|
||
| // fdatasync flushes written data to a file descriptor. | ||
| func fdatasync(db *DB) error { |
There was a problem hiding this comment.
The bbolt already call fsync after grow (truncate the size of file). All the require metadata has been synced.
If the size doesn't change, fdatasync is good enough. The kernel will ensure all the dirty data flushed into disk before return from fdatasync.
There was a problem hiding this comment.
The bbolt already call fsync after grow (truncate the size of file). All the require metadata has been synced.
Basically it's true, the key info filesize should have already been synced before writing any data (not including some unimportant info, e.g. modtime).
If there is no performance difference between fsync and fdatasync given no file size change [will verify it separately], then there is no reason to maintain a separate bolt_linux.go. The goal is to improve the readability & maintainability little by little.
There was a problem hiding this comment.
Based on the man page, https://man7.org/linux/man-pages/man2/fsync.2.html
The aim of fdatasync() is to reduce disk activity for applications that do not require all metadata to be synchronized with the disk.
The fsync might introduce unnecessary metadata related IO. IMO, the access time or modified time is not important in this case. I don't think we should use fsync for each write.
I'm not filesystem developer but for the kernel code, fsync needs to sync for metadata.
// https://elixir.bootlin.com/linux/v6.6.2/source/fs/sync.c#L206
do_fsync(unsigned int fd, int datasync)
|____ vfs_fsync
|_____ vfs_fsync_range
if (!datasync && (inode->i_state & I_DIRTY_TIME))
mark_inode_dirty_sync(inode);
return file->f_op->fsync(file, start, end, datasync);
For the ext4 system with default ordered journaling mode,
it enforces an ordering constraint to guarantee file-system consistency, in which the transaction-related data writes must be completed before the journal writes of the metadata
From https://www.usenix.org/system/files/conference/atc17/atc17-park.pdf
That also means that the fsync has extra write IO.
So, based on my knowledge and documentation, I think we should use fdatasync here.
There was a problem hiding this comment.
Did not get time to verify the performance difference. Not a priority either, so closing this PR for now.
Eventually
file.Sync()callssyscall.Fsync, which flushes both data and metadata. Whilesyscall.Fdatasynconly flushes data. Sofile.Syncis safer and it's has better readability.Reference:
The data sync code was introduced in boltdb/bolt#76 more than 9 years ago. I am not sure why
file.Sync()wasn't used in the first place. Was it for better performance becausesyscall.Fdatasyncdoesn't flush metadata? @benbjohnsoncc @fuweid and @tjungblu