fix(buffers): fsync the disk_v2 data directory on file creation and its newly-created ancestors#25779
Conversation
The disk_v2 buffer synced a new data file's contents to disk but never synced the parent data directory, so the file's directory entry was not made durable. After a crash (e.g. SIGKILL or power loss) a newly created data file could disappear entirely on reopen even though its contents had been fsynced -- taking with it records the buffer had already acknowledged end-to-end to its upstream source, which the source therefore would not retransmit. This is a permanent loss of acknowledged data. The writer now fsyncs the data directory immediately after creating a new (empty) data file, so the file is durably present before any records written into it can be acknowledged. A new `Filesystem::sync_directory` performs the directory fsync on Unix; on non-Unix platforms, where there is no portable way to durably persist directory entries, it warns once and is a no-op. (cherry picked from commit 76e0fb1)
On first startup the buffer creates its data directory chain (`<base>/buffer/v2/<id>`) via create_dir_all. Fsyncing a directory only makes its contents durable, not its own entry in its parent, so the newly created `<id>`, `v2`, and `buffer` directory entries were never made durable -- a power loss after first startup could drop them (and an already-acknowledged data file inside) despite the per-data-file directory fsync. load_or_create now records the deepest pre-existing ancestor before creating the chain, then fsyncs every directory it created, from the buffer directory's parent up to that ancestor, before any write can be acknowledged. Durability across a real crash is validated by the Antithesis suite; like the create-path directory fsync, there is no in-process unit test (a directory entry's durability is only observable across a real crash).
Addresses review of the directory-fsync durability changes:
- sync_directory: treat a directory that does not support fsync
(Unsupported/InvalidInput, seen on some FUSE/overlay/network mounts) as a
warn-once-and-continue rather than propagating a fatal error to the write and
init paths -- the pre-change buffer worked on those filesystems.
- load_or_create: only walk up past a missing ancestor on ErrorKind::NotFound
(not every stat error), so an existing-but-unstattable buffer directory is not
misclassified as freshly created and its pre-existing ancestors re-fsynced.
- load_or_create: skip the empty-path ancestor that parent() yields for a
relative data_dir, which otherwise attempted sync_directory("") and failed
buffer initialization.
- writer roll: fsync the data directory only when a data file was just
atomically created, not on every reopen of an existing empty file.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b557561eb1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // only mean that we're still initializing, and so this would be the | ||
| // data file we left off writing to. | ||
| Some((data_file, file_len)) | ||
| Some((data_file, file_len, false)) |
There was a problem hiding this comment.
When rolling to the next file, if open_file_writable_atomic first returns AlreadyExists but the reader deletes that file before this fallback open_file_writable runs, the fallback's create(true) recreates a new empty data file. This branch then marks just_created as false, so the directory fsync added below is skipped and a crash can still lose the newly-created file after records from it have been acknowledged.
Useful? React with 👍 / 👎.
| self.ledger | ||
| .filesystem() | ||
| .sync_directory(self.ledger.data_dir()) | ||
| .await?; |
There was a problem hiding this comment.
This directory fsync persists every pending mutation in the data directory, not just the new file entry. In a concurrent run, the reader can have completed delete_completed_data_file's delete_file call but not yet incremented/flushed the ledger; if the writer reaches this fsync in that window and the host crashes, the old data-file deletion is durable while the ledger still points at it, which is the deletion-side recovery gap this change explicitly leaves unfixed.
Useful? React with 👍 / 👎.
| let directory = tokio::fs::File::open(path).await?; | ||
| match directory.sync_all().await { |
There was a problem hiding this comment.
On Unix, a deployment can use a data directory that is writable/searchable but not readable (for example mode 0300); Vector's data-dir validation only requires writability, and creating the known buffer files works in that directory. This new directory fsync path opens the directory read-only before it reaches the unsupported-fsync downgrade, so those previously valid disk buffers now fail with PermissionDenied during startup or rotation instead of merely warning that the directory entry cannot be made durable.
Useful? React with 👍 / 👎.
1 participant
Summary
Two related crash-durability fixes for the
disk_v2buffer's directory handling. Fsyncing a file makes its contents durable but not its directory entry; a directory entry only becomes durable when the containing directory itself is fsynced. Both fixes close windows where a crash could make a data file — and records already acknowledged from it — silently disappear on reopen.Data file creation. When creating a new
.datdata file the buffer synced the file's contents but not the parent data directory, so after a crash the newly created file could be absent on reopen despite its contents being fsynced. The buffer now fsyncs the data directory immediately after creating a data file, before any record written into it can be acknowledged.Buffer directory ancestors (first startup). On first startup the buffer creates its data directory chain (
<base>/buffer/v2/<id>) withcreate_dir_all, which does not make those new directory entries durable in their parents. A power loss right after first startup could drop the freshly created<id>/v2/bufferdirectories and an already-acknowledged data file inside them.load_or_createnow records the deepest pre-existing ancestor, then fsyncs each directory it created — from the buffer directory's parent up to that ancestor — before any write can be acknowledged.A third commit hardens these against edge cases surfaced in review: directory-fsync that the filesystem doesn't support (
Unsupported/InvalidInput, seen on some FUSE/overlay/network mounts) is downgraded to a warn-once instead of a fatal error; the ancestor walk only climbs past a missing directory onNotFound(not any stat error); the empty-path ancestor of a relativedata_diris skipped; and the create-path fsync fires only when a file was actually just created (not on reopen of an existing empty file).Scope
This PR covers only the creation side of directory durability. The mirror fix for data-file deletion, and the reader-side crash recovery it requires, are intentionally not included here — they depend on broader disk-buffer recovery work and will land with it.
How did you test this PR?
cargo check -p vector-buffers. These are crash-window durability guarantees observable only across a real crash, so they are validated by the crash-injecting Antithesis suite rather than by in-process unit tests (consistent with the existing directory-fsync code).Change Type
Is this a breaking change?
Does this PR include user facing changes?