Skip to content

Write file:// remote cache blobs atomically - #23728

Merged
benjyw merged 3 commits into
pantsbuild:mainfrom
aegis-cipherowl:henry-yang/upstream/file-store-atomic-writes
Nov 9, 2026
Merged

benjyw merged 3 commits into
pantsbuild:mainfrom
aegis-cipherowl:henry-yang/upstream/file-store-atomic-writes

Conversation

@aegis-cipherowl

Copy link
Copy Markdown
Contributor

Background

The experimental-file remote cache provider (remote_store_address = "file://...") builds opendal's Fs service without atomic_write_dir. In that mode opendal opens each blob's final path with O_TRUNC and writes into it in place (via tokio::io::copy, in 8 KiB chunks). If the write is interrupted, for example because Pants is killed mid-upload, a truncated blob is left under its digest's name:

  • list_missing_digests reports the digest as present, so it is never re-uploaded.
  • load validates the digest and fails with Remote CAS gave wrong digest, but never removes the blob, so the store stays broken until the file is deleted by hand.
  • abort is unsupported in this mode, so a failed upload can't be cleaned up either.

We hit this in practice: every bad blob in our stores was truncated at an 8 KiB boundary.

Change

Provider::fs sets atomic_write_dir to a .tmp directory under the store root. opendal then writes each blob to a temporary file and renames it to its final path once complete, so readers see either the whole blob or nothing. The directory is under the root so the rename never crosses a filesystem boundary. opendal already called sync_all on every write, so there is no extra fsync.

If a process dies mid-write, its temporary file is left in .tmp. That is harmless, but nothing currently cleans it up. Blobs that are already truncated are not repaired; the release note says to delete them.

Tests

  • fs_partial_write_is_not_visible: starts a write, stops partway, and checks the blob is neither on disk under its final path nor loadable, and is still reported missing. It then finishes the write and checks the whole blob appears. This fails without the fix (assertion failed: !root.path().join(&path).exists()).
  • fs_store_file_round_trip: store_file against a real directory succeeds and leaves no temporary files behind.
  • cargo test -p remote_provider_opendal passes (19 tests). cargo clippy and cargo fmt are clean.

LLM assistance notice

This PR was written primarily by Claude Code (Claude Opus 5.5). I directed the scope and reviewed the result.

🤖 Generated with Claude Code

The `experimental-file` remote cache provider built opendal's Fs service
without `atomic_write_dir`, so opendal opened each blob's final path with
O_TRUNC and wrote into it in place. An interrupted write (e.g. a killed
Pants process) left a truncated blob under its digest's name. Reads detect
the wrong digest, but never remove the blob, so the store stays broken until
the file is deleted by hand.

Set `atomic_write_dir` to a `.tmp` directory inside the store root, so each
blob is written to a temporary file and renamed into place once complete.
Keeping it under the root means the rename never crosses filesystems.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aegis-cipherowl

Copy link
Copy Markdown
Contributor Author

Performance

I measured the write cost with and without atomic_write_dir, using Provider itself (store_bytes / store_file) against a temp directory on an APFS SSD (macOS, Apple Silicon). Each case ran 5 times, alternating which mode went first; the table shows medians. Blob sizes follow a real file:// cache of 61k blobs (median 1.2 KiB, 79% ≤ 8 KiB, 1% > 1 MiB).

Per-blob time:

Workload Without With, opendal 0.58 (this PR) With, opendal 0.53
1.2 KiB store_bytes, sequential 5.38 ms 5.46 ms (+1.6%) 5.58 ms (+4.4%)
1.2 KiB store_bytes, 32 concurrent 4.81 ms 4.89 ms (+1.7%) 5.05 ms (+5.6%)
20 KiB store_bytes, 32 concurrent 4.93 ms 5.17 ms (+5.0%) 5.19 ms (+3.7%)
1 MiB store_file, 8 concurrent 7.89 ms 8.37 ms (+6.1%) 8.13 ms (−9.7%)
16 MiB store_file, 4 concurrent 54.3 ms 54.6 ms (+0.5%) 59.8 ms (+5.0%)

The "Without" column is from the opendal 0.58 runs; each percentage compares against the "Without" run on the same opendal version. On 0.53, the "Without" times were 5.34, 4.79, 5.00, 9.01 and 56.9 ms.

The overhead is roughly 0.1–0.3 ms per blob (about 2–6%), and the run-to-run ranges overlap. The 0.53 1 MiB case even came out faster with temp files.

Almost all of each write's cost is the same in both modes. FsWriter::close always calls sync_all, which Rust's std implements as fcntl(F_FULLFSYNC) on Apple platforms. A standalone test of the individual steps (1.2 KiB files):

Per-file steps Cost
write 144 µs
write + rename 145 µs
write + F_FULLFSYNC 4,775 µs
write + F_FULLFSYNC + rename (the new path) 5,328 µs

The rename itself is free. The extra cost appears only after a full flush, presumably because the rename then has to wait on the filesystem journal.

That flush also limits this provider to about 200 blob writes per second regardless of concurrency. That's a separate, pre-existing cost that this PR neither adds nor removes. I haven't measured Linux, where sync_all is a plain fsync.

There's no prior discussion of this setting on the Pants side. #19827 introduced the provider without it, and later changes to Provider::fs were only opendal upgrades. opendal added the option as opt-in in response to apache/opendal#788, which describes this exact truncated-file problem. apache/opendal#8315 (open) would add a parent-directory fsync after the rename, so this should be re-measured if that lands.

(Measured by Claude Code; the benchmark was a temporary #[ignore] test and isn't part of this PR.)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@benjyw
benjyw merged commit 94f4527 into pantsbuild:main Nov 9, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants