Skip to content

[storage] use Blob::write_at_sync#3858

Merged
patrick-ogrady merged 12 commits into
mainfrom
andre/storage-write-at-sync
May 22, 2026
Merged

[storage] use Blob::write_at_sync#3858
patrick-ogrady merged 12 commits into
mainfrom
andre/storage-write-at-sync

Conversation

@andresilva
Copy link
Copy Markdown
Member

@andresilva andresilva commented May 22, 2026

This PR updates Metadata full rewrites to use Blob::write_at_sync when the new metadata payload does not shrink the blob. A non-shrinking rewrite is a single durable write, so it no longer needs a separate sync. Shrinking rewrites still use write_at + resize + sync because the resize must also be made durable.

The rest of the changes replace manual put/write_at followed by sync with the corresponding sync helper where that is equivalent and clearer. This includes uses of Metadata::put_sync, Archive::put_sync, Cache::put_sync, and direct Blob::write_at_sync in tests.

Most storage primitives already benefit from write_at_sync indirectly through the runtime buffering layers. buffer::Write and buffer::paged::Append can use write_at_sync when flushing a single write with no earlier dirty mutation, so databases built on journals, ordinal storage, cache/archive paths, and QMDB logs inherit that behavior.

Depends on #3857.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 22, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
commonware-mcp 80c2480 May 22 2026, 08:51 PM

@andresilva andresilva moved this to Ready for Review in Tracker May 22, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 22, 2026

Benchmark results

Tip

PASSED: No benchmark exceeded the regression threshold.

Benchmark comparison table
Benchmark Baseline (main) Current Delta Threshold Status
qmdb::merkleize/variant=any::unordered::fixed::mmr keys=10000 ch=false sync=false 1.714 ms 1.629 ms -4.97% 10.00% ✅ PASS
qmdb::merkleize/variant=current::ordered::fixed::mmb chunk=256 keys=10000 ch=true sync=false 3.180 ms 3.139 ms -1.29% 10.00% ✅ PASS

Baseline commit(s): 2ac16955cd92

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates storage primitives and runtime buffering layers to use per-write durability helpers (Blob::write_at_sync, Metadata::put_sync, Archive::put_sync, Cache::put_sync) where a single write can be made durable without requiring a separate full sync(). This reduces unnecessary durability barriers while preserving correct behavior for cases that still require a full sync (notably shrink/resizes and multi-write flushes).

Changes:

  • Update Metadata full-rewrite sync logic to use Blob::write_at_sync for non-shrinking rewrites, while retaining write_at + resize + sync for shrinking rewrites.
  • Replace multiple put/write_at + sync call pairs across storage code (including tests and fuzz target corruption steps) with the corresponding *_sync helpers.
  • Enhance runtime buffer implementations (buffer::Write and buffer::paged::Append) with explicit “needs full sync” tracking to safely use write_at_sync only when no prior unsynced mutation exists; add targeted tests validating durability-barrier behavior.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file
File Description
storage/src/qmdb/keyless/compact.rs Use Metadata::put_sync in tests instead of put + sync.
storage/src/qmdb/immutable/compact.rs Use Metadata::put_sync in tests instead of put + sync.
storage/src/ordinal/mod.rs Use Blob::write_at_sync in corruption tests to persist single writes without extra sync.
storage/src/metadata/storage.rs Optimize full rewrites: non-shrinking rewrites use write_at_sync; shrinking rewrites still require resize + full sync.
storage/src/metadata/mod.rs Update tests to use write_at_sync / put_sync helpers for clearer durable writes.
storage/src/merkle/persisted/full.rs Replace metadata put + sync with put_sync where equivalent; update test accordingly.
storage/src/merkle/persisted/compact.rs Replace metadata put + sync with put_sync in implementation and tests.
storage/src/journal/segmented/variable.rs Use write_at_sync in corruption tests; adjust one test payload to force a physical resize path.
storage/src/journal/segmented/oversized.rs Use write_at_sync in corruption tests to persist single writes without extra sync.
storage/src/journal/contiguous/fixed.rs Use put_sync for durable metadata writes; ensure remove path still performs metadata.sync().
storage/src/freezer/storage.rs Use write_at_sync in corruption test for a single durable write.
storage/src/freezer/mod.rs Use write_at_sync in corruption tests to persist single writes without extra sync.
storage/src/archive/immutable/storage.rs Publish checkpoint via metadata.put_sync (single durable op) after underlying state is durable.
storage/src/archive/immutable/mod.rs Use Archive::put_sync in test instead of put + sync.
storage/fuzz/fuzz_targets/oversized_recovery.rs Persist fuzz corruption writes with write_at_sync instead of write_at + sync.
runtime/src/utils/buffer/write.rs Introduce shared writer State tracking prior unsynced mutations to safely use write_at_sync on single-write flushes.
runtime/src/utils/buffer/paged/append.rs Add needs_sync tracking and conditional write_at_sync usage; refine shrink behavior; add durability-focused tests.
runtime/src/utils/buffer/mod.rs Add SyncTrackingBlob test helper and new tests asserting correct range-sync vs full-sync selection.

# Conflicts:
#	runtime/src/utils/buffer/mod.rs
#	runtime/src/utils/buffer/paged/append.rs
#	runtime/src/utils/buffer/write.rs
#	storage/src/journal/segmented/variable.rs
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 22, 2026

Deploying monorepo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 80c2480
Status: ✅  Deploy successful!
Preview URL: https://96576614.monorepo-eu0.pages.dev
Branch Preview URL: https://andre-storage-write-at-sync.monorepo-eu0.pages.dev

View logs

Copy link
Copy Markdown
Contributor

@patrick-ogrady patrick-ogrady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comments

Comment thread storage/src/journal/segmented/variable.rs Outdated
Comment thread storage/src/metadata/mod.rs
@patrick-ogrady
Copy link
Copy Markdown
Contributor

bugbot run

@patrick-ogrady patrick-ogrady marked this pull request as ready for review May 22, 2026 20:51
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 619c771. Configure here.

@patrick-ogrady patrick-ogrady merged commit a7e8f48 into main May 22, 2026
175 of 176 checks passed
@patrick-ogrady patrick-ogrady deleted the andre/storage-write-at-sync branch May 22, 2026 20:56
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Tracker May 22, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

❌ Patch coverage is 94.44444% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.77%. Comparing base (2ac1695) to head (80c2480).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
storage/src/journal/contiguous/fixed.rs 70.00% 6 Missing ⚠️
@@            Coverage Diff             @@
##             main    #3858      +/-   ##
==========================================
- Coverage   95.77%   95.77%   -0.01%     
==========================================
  Files         486      486              
  Lines      201251   201235      -16     
  Branches     4876     4876              
==========================================
- Hits       192754   192730      -24     
- Misses       6862     6866       +4     
- Partials     1635     1639       +4     
Files with missing lines Coverage Δ
storage/src/archive/immutable/mod.rs 100.00% <100.00%> (ø)
storage/src/archive/immutable/storage.rs 90.57% <100.00%> (+0.04%) ⬆️
storage/src/freezer/mod.rs 99.14% <100.00%> (-0.01%) ⬇️
storage/src/freezer/storage.rs 92.38% <100.00%> (-0.02%) ⬇️
storage/src/journal/segmented/oversized.rs 98.66% <100.00%> (-0.01%) ⬇️
storage/src/journal/segmented/variable.rs 93.91% <100.00%> (-0.03%) ⬇️
storage/src/merkle/persisted/compact.rs 94.08% <100.00%> (+0.06%) ⬆️
storage/src/merkle/persisted/full.rs 96.82% <100.00%> (+<0.01%) ⬆️
storage/src/metadata/mod.rs 99.86% <100.00%> (+<0.01%) ⬆️
storage/src/metadata/storage.rs 90.70% <100.00%> (ø)
... and 4 more

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ac1695...80c2480. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants