crepererum commented on code in PR #643:
URL:
https://github.com/apache/arrow-rs-object-store/pull/643#discussion_r3044148737
##########
src/local.rs:
##########
@@ -786,23 +855,69 @@ impl LocalFileSystem {
}
/// Creates the parent directories of `path` or returns an error based on
`source` if no parent
-fn create_parent_dirs(path: &std::path::Path, source: io::Error) -> Result<()>
{
+///
+/// When `fsync` is true, fsyncs each newly created directory and the first
pre-existing
+/// ancestor to ensure the new directory entries are durable.
+fn create_parent_dirs(path: &std::path::Path, source: io::Error, fsync: bool)
-> Result<()> {
let parent = path.parent().ok_or_else(|| {
let path = path.to_path_buf();
Error::UnableToCreateFile { path, source }
})?;
- std::fs::create_dir_all(parent).map_err(|source| {
- let path = parent.into();
- Error::UnableToCreateDir { source, path }
+ if fsync {
+ let mut first_existing = parent;
+ while !first_existing.exists() {
+ first_existing = first_existing.parent().unwrap_or(first_existing);
+ }
+
+ std::fs::create_dir_all(parent).map_err(|source| {
+ let path = parent.into();
+ Error::UnableToCreateDir { source, path }
+ })?;
Review Comment:
If you pull this create_dir_all before the `if fsync`, then it's the same
code for fsync and no-fsync. That makes it easier to reasons about, since it's
really just the fsync-block that differs, not the "actual" I/O operation.
##########
src/local.rs:
##########
@@ -882,11 +1000,21 @@ impl MultipartUpload for LocalUpload {
async fn complete(&mut self) -> Result<PutResult> {
let src = self.src.take().ok_or(Error::Aborted)?;
let s = Arc::clone(&self.state);
+ let fsync = self.fsync;
maybe_spawn_blocking(move || {
// Ensure no inflight writes
let file = s.file.lock();
+ if fsync {
Review Comment:
why is there an fsync before this rename but not before the one in
`rename_opts`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
