Skip to content

[storage] Expand CRC Fix Scope#3839

Closed
patrick-ogrady wants to merge 8 commits into
codex/append-resize-crc-fallbackfrom
expand-crc-fix
Closed

[storage] Expand CRC Fix Scope#3839
patrick-ogrady wants to merge 8 commits into
codex/append-resize-crc-fallbackfrom
expand-crc-fix

Conversation

@patrick-ogrady
Copy link
Copy Markdown
Contributor

@patrick-ogrady patrick-ogrady commented May 20, 2026

#3837 fixes the original same-page partial shrink case: an already-persisted partial tail can be resized to a shorter partial length without making the page unrecoverable if the CRC rewrite is interrupted.

This PR covers the related cases that #3837 does not cover: shrink targets that land inside a page which was previously full, and therefore was not represented by partial_page_state.

Concretely, this adds crash-safety for:

  • full page -> partial page, for example PAGE_SIZE -> 45
  • multi-page shrink landing inside a previously full page, for example 3 * PAGE_SIZE -> PAGE_SIZE + 45

Those target pages already have valid CRC records on disk, but #3837's protected path only ran when the target page was the current persisted partial tail. Without this PR, the fallback path can later rewrite the landing page as a padded partial page; if that write partially lands before the shorter CRC is durable, recovery can reject the landing page and truncate to the previous page boundary.

This PR fixes that by reading and validating the target page to recover the CRC record that actually committed it, then using the same two-phase CRC transition for any shrink whose target has nonzero partial bytes. If later physical pages need to be truncated first, that truncation is synced before the landing page's shorter CRC is staged.

The implementation also removes the now-unreachable later partial-tail reload path: after the CRC-safe partial_bytes > 0 branch returns, the remaining resize path is page-aligned shrink only.

Testing:

  • Adds full-page-to-partial interrupted CRC stage/invalidation regressions.
  • Adds multi-page-to-partial interrupted CRC stage/invalidation regressions.
  • Verified with just test -p commonware-runtime resize_same_page_shrink_survives_interrupted_crc_stage resize_same_page_shrink_survives_interrupted_crc_invalidation resize_full_page_to_partial resize_multi_page_to_partial.

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

cloudflare-workers-and-pages Bot commented May 20, 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 6177fca May 20 2026, 07:11 AM

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 20, 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.567 ms 1.581 ms +0.91% 10.00% ✅ PASS
qmdb::merkleize/variant=current::ordered::fixed::mmb chunk=256 keys=10000 ch=true sync=false 3.100 ms 3.063 ms -1.20% 10.00% ✅ PASS

Baseline commit(s): 14a016a55f43

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

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

Deploying monorepo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6177fca
Status: ✅  Deploy successful!
Preview URL: https://daabbf19.monorepo-eu0.pages.dev
Branch Preview URL: https://expand-crc-fix.monorepo-eu0.pages.dev

View logs

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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit fb29647. Configure here.

Comment thread runtime/src/utils/buffer/paged/append.rs Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 98.23009% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.74%. Comparing base (fd03283) to head (6177fca).

Files with missing lines Patch % Lines
runtime/src/utils/buffer/paged/append.rs 98.07% 1 Missing and 1 partial ⚠️
@@                        Coverage Diff                        @@
##           codex/append-resize-crc-fallback    #3839   +/-   ##
=================================================================
  Coverage                             95.73%   95.74%           
=================================================================
  Files                                   473      473           
  Lines                                192765   192841   +76     
  Branches                               4676     4672    -4     
=================================================================
+ Hits                                 184548   184630   +82     
+ Misses                                 6645     6642    -3     
+ Partials                               1572     1569    -3     
Files with missing lines Coverage Δ
runtime/src/utils/buffer/paged/cache.rs 94.06% <100.00%> (ø)
runtime/src/utils/buffer/paged/mod.rs 97.02% <100.00%> (+0.04%) ⬆️
runtime/src/utils/buffer/paged/append.rs 97.46% <98.07%> (+0.31%) ⬆️

... 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 fd03283...6177fca. 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.

@roberto-bayardo
Copy link
Copy Markdown
Collaborator

have added this extension of the fix to my PR (plus some other stuff).

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