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 sync call #136

Merged
merged 2 commits into from
Mar 28, 2024
Merged

Add sync call #136

merged 2 commits into from
Mar 28, 2024

Conversation

mulimoen
Copy link
Collaborator

Fixes #135

@@ -423,6 +423,17 @@ impl FileMut {
let (ncid, name) = super::group::get_parent_ncid_and_stem(self.ncid(), name)?;
super::variable::add_variable_from_identifiers(ncid, name, dims, T::NCTYPE)
}

/// Flush pending buffers to disk to minimise data loss in case of termination.
Copy link
Member

@lnicola lnicola Mar 28, 2024

Choose a reason for hiding this comment

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

I don't really have an HPC background, but I have trouble imagining I could trust the contents of a file after a crash without at least some kind of logging scheme.

But this is also useful to detect I/O errors that happen when closing the file, since drop isn't going to return anything.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's some good points you're raising. This was taken from the official documentation, and should cover crashes outside of netCDF (e.g. when computing data).

Copy link
Member

@lnicola lnicola Mar 28, 2024

Choose a reason for hiding this comment

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

https://danluu.com/file-consistency/ is worth a read by anyone feeling lucky enough to try to "minimize data loss" this way (of course, app crashes different from system crashes).

@lnicola
Copy link
Member

lnicola commented Mar 28, 2024

Good idea adding close :-).

@mulimoen mulimoen merged commit 35f4555 into georust:master Mar 28, 2024
13 checks passed
@mulimoen mulimoen deleted the feature/sync branch March 28, 2024 15:02
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.

Flushing file
2 participants