Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add safe file writer #48

Merged
merged 8 commits into from
Aug 7, 2024
Merged

add safe file writer #48

merged 8 commits into from
Aug 7, 2024

Conversation

elv-gilles
Copy link
Collaborator

@elv-gilles elv-gilles commented Jul 16, 2024

Add safe file writing using implementation taken from https://github.com/google/renameio.

Given the potential performance impact, this implementation allows to by-pass the call to fsync.

sync := false
wr, err := fileutil.NewSafeWriter(f.path, fileutil.WithSyncBeforeRename(sync))

lukseven and others added 5 commits July 1, 2024 20:09
* log warn if waiting to obtain the lock
* retry renaming & log warn when 'finalizing' a safe write
* add func 'LockCount' to namedLocks
* discard previous implementation
elv-nate
elv-nate previously approved these changes Jul 23, 2024
Copy link
Contributor

@elv-nate elv-nate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This seems robust, even though I do think it could be a good idea to add randomness so that multiple temp files can exist concurrently, as this implementation could result in partial files if multiple writers use a safe writer, which isn't necessarily the best behavior.

util/fileutil/safe_writer.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@lukseven lukseven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am probably missing something, but where is the reader?

util/fileutil/safe_writer.go Outdated Show resolved Hide resolved
util/fileutil/safe_writer.go Outdated Show resolved Hide resolved
util/fileutil/safe_writer.go Show resolved Hide resolved
@elv-gilles
Copy link
Collaborator Author

I am probably missing something, but where is the reader?

There's no specific reader: opening the file and reading from it gives the last successful version of it.

@elv-gilles elv-gilles merged commit e98a3af into master Aug 7, 2024
1 check passed
@elv-gilles elv-gilles deleted the safe-file2 branch August 7, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants