🐛 fix(storage): issue no receipt when a directory flush fails - #1999
Merged
Conversation
The filesystem durability boundary discarded every parent-directory failure: sync_parent ignored both the directory open and its sync_all, so commit_staged went on to hand out a DurabilityCapabilities::FILESYSTEM receipt for a rename that may never have reached disk. Replication and acknowledgement code then weighed that receipt as proof of a directory entry a crash could still lose. The helper now returns the failure and every publication path propagates it, so a refused flush produces an I/O error and no receipt. A fresh digest fan-out flushes each directory it creates from the leaf toward the first pre-existing level, since create_dir_all leaves each new level as an unflushed entry in its own parent. A commit that finds matching bytes already resident still flushes: those bytes may come from a writer whose own sync failed, so their presence says nothing about the durability of the entry naming them. That also lets a retry after a successful rename finish the durability step without touching the resident blob. Unix uses directory fsync. Windows exposes no directory flush, so it keeps relying on the NTFS metadata log that orders the rename, matching the operator restore path.
Merging this PR will not alter performance
Comparing Footnotes
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A local-filesystem commit issued a
DurabilityCapabilities::FILESYSTEMreceipt whether or not the directory entry naming the blob ever reached disk.sync_parentswallowed the directory open behindif let Ok(...)and itssync_allbehindlet _ =, sopublishreturned success after the kernel refused the flush andcommit_stagedhanded back a receipt. Replication and acknowledgement then weighed that receipt as proof of an entry a crash can still lose. #516 requires the parent-directory sync before any receipt, and the POSIXfsynccontract says why.The helper returns its failure now and every publication path propagates it, so a refused flush ends as an I/O error with no receipt. 🔒 Two cases needed more than plumbing. A fresh store has no digest fan-out, and
create_dir_allleaves each level it creates as an unflushed entry in its own parent, so the store flushes those new levels from the leaf toward the first that already existed. A commit that finds matching bytes already resident flushes too: those bytes can come from a writer whose own sync failed, so their presence proves nothing about the entry naming them. That second rule is what lets a retry after a successful rename finish the durability step without touching the resident blob, the resumption path #1501 builds on.flowchart LR Stage[Stage synced] --> Rename[Rename into digest path] Rename --> Flush{Directory flush} Flush -- ok --> Receipt[Placement receipt] Flush -- refused --> Error[I/O error, no receipt] classDef accent fill:#cfe4ff,stroke:#1f6feb,color:#0b1f3a; classDef warn fill:#ffe3a3,stroke:#d29200,color:#3a2c00; class Stage,Rename,Receipt accent; class Flush,Error warn;Unix flushes the directory with
fsync. Windows keeps the behavior it had, since it exposes no directory flush and NTFS orders the rename in the metadata log its recovery pass replays, which is whatcrates/peryx/src/operator/restore.rsalready relies on. The issue also asked for a documented Win32 write-through path; I left that out rather than guess, becauseFlushFileBuffersrejects a handle opened with backup semantics.Two behavior changes to watch. A store whose directories the process cannot open for reading now fails its commits instead of returning a receipt for a flush that never happened. A deduplicating write pays one extra directory
fsync.Closes #1391