Pre-allocate the output file on Darwin to avoid APFS sparse-file corruption#361
Merged
Conversation
AssembleFile truncated the output file to its target size, which on APFS creates a sparse file. Concurrent WriteAt calls on adjacent regions of large sparse files have shown non-deterministic corruption there, with null-chunk regions occasionally reading back wrong (#326). Replace the plain truncate with preallocateFile: on Darwin it uses fcntl(F_PREALLOCATE) to physically allocate blocks before truncating. Since F_PREALLOCATE with F_PEOFPOSMODE allocates relative to the current end of file, only the missing difference is requested, keeping in-place reassembly of existing files from over-allocating. Filesystems without F_PREALLOCATE support (SMB, FUSE) fall back to the previous plain-truncate behavior. All other platforms delegate to Truncate as before.
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.
Combines #326 by @nsakaimbo with the review fixes from nsakaimbo#1 into a single PR. Supersedes #326.
Problem
On macOS/APFS,
os.Truncatecreates a sparse file. WhenAssembleFile's concurrent workersWriteAtadjacent regions of a large sparse file, the assembled output has shown non-deterministic corruption — correct size, but null-chunk regions occasionally read back wrong (reported in #326 with ~350 GB VM disk images on M1, where forcing read-back verification made the problem disappear). Physically pre-allocating the blocks before assembly eliminates it, at no cost to assembly time.Changes
AssembleFilenow callspreallocateFileinstead ofos.Truncatefor regular file targets. Block devices are unaffected.preallocate_darwin.go): usesunix.FcntlFstorewithF_PREALLOCATE/F_ALLOCATEALLto physically allocate blocks, then truncates to the final size.F_PEOFPOSMODEallocates relative to the current end of file, so only the missing difference (size - current) is requested. This keeps in-place reassembly of an existing file from allocating the full size again beyond EOF (which could spuriously fail withENOSPC), and skips the fcntl entirely when the file doesn't need to grow.ENOTSUP— not all filesystems supportF_PREALLOCATE(SMB, some FUSE mounts), and the sparse-hole issue is APFS-specific.preallocate_other.go): delegates toTruncate, no behavior change.nullChunkSeed.LongestMatchWithis kept. It was added in Do not immediately abort the extraction if a seed chunk is invalid #220 for worker load-balancing, not correctness: chunks beyond the limit are picked up by the nextLongestMatchWithcall or the store, and removing it would make a single worker serially zero-fill unbounded null runs during in-place assembly on non-reflink filesystems. The pre-allocation alone addresses the corruption.Tests
preallocate_test.go(all platforms): new, grown (original content preserved, tail reads as zeros), shrunk, same-size, and empty files.preallocate_darwin_test.go: regression test for the delta calculation — re-preallocates an existing file to its current size on a nearly-full APFS volume created withhdiutil, which fails withENOSPCif the full size is requested instead of the difference.One behavior change to be aware of: with
F_ALLOCATEALL, extracting a mostly-zero image on macOS now requires the full physical space up front rather than succeeding sparsely. That matches the intent of the existing "confirm there's enough disk space" comment.