reidkaufmann opened a new pull request, #890: URL: https://github.com/apache/arrow-rs-object-store/pull/890
# Which issue does this PR close? - Part of #888. # Rationale for this change The first two interleavings in the issue: `create_parent_dirs` fsyncs from the new parent up to the first ancestor a snapshot taken before `create_dir_all` found existing, so a write into a directory another write is still creating, or a concurrent delete with automatic cleanup, can leave a new directory's entry unsynced while the write returns success. # What changes are included in this PR? - A process-wide registry (`parking_lot` mutex and condvar) of the directories a call is creating; `fsync_parent_dir` waits while an ancestor of its path is registered. Cost with nothing registered: one acquisition of the process-wide lock per durable write (two for a cross-directory rename) and no walk; the ancestor walk runs only while a creation is registered. With `fsync` disabled nothing changes. - `create_parent_dirs` creates the missing components one `create_dir` at a time, registering each before trying it, and fsyncs every registered directory and the one holding the highest one's entry. That covers what the snapshot found missing, what cleanup removed in between, and a directory another creator (another process, a store without fsync) made in between, which `main` also synced. - The creation and sync functions are parameters of the new `create_dir_all_durable`; production passes `std::fs::create_dir` and `fsync_dir`, tests pass closures that interleave the other party and record the sync order. No test hooks in production code. The registry is keyed by path within one process because several `LocalFileSystem` instances can share a root; other processes, aliased paths and stores sharing the root with `fsync` disabled are not coordinated, which `with_fsync` now documents. The registry also runs on Windows, where `fsync_dir` is a no-op. A write waits without timeout while an ancestor is being fsynced by another call. Not changed: a delete with automatic cleanup can still remove a directory between a creator's `create_dir` and its `fsync_dir`; the creator then returns an error, as before. #866 replaces `delete_location`'s `fsync_parent_dir` call with a pre-opened parent handle, so with both merged a delete does not wait on the registry; the two change different hunks. Tests, and how each fails without the fix: - `fsync_write_waits_for_in_progress_dir_creation` registers a directory in the registry directly, because a real race cannot be forced deterministically; without the wait in `fsync_parent_dir` the write into the registered directory finishes during the 100 ms and the test panics with "returned before `a` was durable". - `fsync_creation_syncs_an_ancestor_removed_by_concurrent_cleanup`: the `create` closure deletes `a` before the first `create_dir`; the snapshot walk syncs `b` and `a` but not `root`. - `fsync_creation_syncs_the_parent_of_a_directory_another_creator_made`: the closure creates `a` right before this call's first `create_dir(a/b)`; syncing only what this call created gives `[a/b, a]`, without `root`. Locally: `cargo fmt --check`, the CI clippy matrix with `-D warnings`, `cargo test`, `cargo test --all-features`, rustdoc with `-D warnings`, `cargo +1.85.0 check --all-targets --all-features`; `local::tests` ten times in a row. Not run on Windows, and not tested under real power loss. # Are there any user-facing changes? With fsync enabled, a write into a directory another write is still creating waits for that creation to finish syncing. `with_fsync` documents the per-process, per-path coordination. No API changes. # AI usage Written with AI assistance; I reviewed the changes and tests. -- 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]
