Add WithSyncDirectory to optionally fsync the directory after the rename#42
Add WithSyncDirectory to optionally fsync the directory after the rename#42Jille wants to merge 1 commit into
Conversation
|
Greetings from quis@ :) |
| } | ||
| t.done = true | ||
| if t.syncDirectory { | ||
| dh, err := os.OpenFile(filepath.Dir(t.path), os.O_WRONLY, 0) |
There was a problem hiding this comment.
On Linux this doesn't work. (Error: open <file>: is a directory)
Simply os.Open(filepath.Dir(t.path)) does work instead.
See also:
-
https://linux.die.net/man/2/fsync
On some UNIX systems (but not Linux), fd must be a writable file descriptor.
-
https://austin-group-l.opengroup.narkive.com/vC4Fjvsn/fsync-ing-a-directory-file-descriptor
On Linux, you would
typically open in read-only (O_RDONLY) the directory (O_WRONLY would
fail otherwise), and call fsync() on the returned file descriptor.
There was a problem hiding this comment.
This is a good point — could you add a test so that the feature is exercised in the CI pipeline?
stapelberg
left a comment
stapelberg
left a comment
There was a problem hiding this comment.
Sorry for letting this PR linger for so long. It’s been a busy time recently.
The reason why the renameio package currently doesn’t provide this feature is so that the user has more control over when a sync happens (as applications often write many files), but I don’t mind adding this for use-cases where only a single file needs to be written reliably.
| } | ||
| t.done = true | ||
| if t.syncDirectory { | ||
| dh, err := os.OpenFile(filepath.Dir(t.path), os.O_WRONLY, 0) |
There was a problem hiding this comment.
This is a good point — could you add a test so that the feature is exercised in the CI pipeline?
3 participants
WithSyncDirectory configures renameio to fsync the directory after renaming
the file. If that succeeds it's guaranteed the file will be in the new state
even after a crash.