GNOME Bugzilla – Bug 575555
Use fsync() when replacing files to avoid data loss on crash
Last modified: 2018-05-24 11:48:42 UTC
We should at the very least fsync when actually replacing a file, otherwise we risk losing both the old and new file if the system crashes. This is somewhat overkill as we don't actually need the changes on disk immediately, but better safe than sorry, and file replaces should not be that common.
Created attachment 130750 [details] [review] Minimal patch to fsync on file replace Optionally we could also add G_FILE_CREATE_SYNC_ON_CLOSE to GFileCreateFlags so that apps that really want to sync can do that.
Sounds good to me. Should the kernel ever grow a less expensive api that lets us specify the ordering constraints we need, we can switch to that. Should probably keep this bug open for the G_FILE_CREATE_SYNC_ON_CLOSE flag addition in 2.21.x
Patch commited.
Alex, did you want to add G_FILE_CREATE_SYNC_ON_CLOSE ?
Created attachment 169294 [details] [review] gio: Allow to not use fsync when replacing a file. Some filesystems _do_ guarantee that the data is written before the metadata. For these, this fsync can be a performance problem. Allow passing a flag to disable that fsync call.
Review of attachment 169294 [details] [review]: But how would an app know this? Should every app be parsing /etc/fstab and looking at the ext{3,4} mount options? Better to have OS-specific code in GIO which has reasonable defaults.
Review of attachment 169294 [details] [review]: More precisely, I'm suggesting gio have a whitelisted set of OS configurations. It may *also* make sense for an app to have a way to opt-out of this though...I'm not sure.
I agree a whitelisted set of configurations would but nicer, but having to lookup the configuration of the filesystem on which a file resides each time we do a replace might also be expensive. Is there a way to way to get the mount flags without parsing /proc/mounts? Parsing fstab seems fragile, as you could be mounting stuff manually, or have stuff mounted on hotplug
Well, maybe the kernel should export API for this; there's even POSIX API for getting random filesystem stuff: fpathconf(). fpathconf(fd, _LINUX_FILESYSTEM_REQUIRES_FSYNC_AFTER_RENAME_TO_NOT_EAT_YOUR_DATA) ?
I'd like to see the G_FILE_CREATE_NO_FSYNC patch get in. Opting out of fsync-on-close rather than opting in is the right way to go for backward compatibility. The flag would be useful when saving non-critical data to a local file. This issue strikes me as a performance versus risk tradeoff and apps are perfectly capable of deciding that for themselves.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/207.