Skip to content

[storage/qmdb] Coordinate state sync finalization#3439

Merged
clabby merged 3 commits into
mainfrom
cl/coordinate-sync
Mar 23, 2026
Merged

[storage/qmdb] Coordinate state sync finalization#3439
clabby merged 3 commits into
mainfrom
cl/coordinate-sync

Conversation

@clabby
Copy link
Copy Markdown
Collaborator

@clabby clabby commented Mar 20, 2026

Overview

Adds explicit finish coordination to QMDB's state sync engine and exposes reached-target notifications so external orchestration can coordinate multiple databases syncing towards a unified set of targets.

Motivation

In #3381, we store the state root and range of each managed database in each block that is formed. When state syncing multiple databases, we must finish state sync for each DB at the same point. To synchronize these tasks, we need:

  1. A way for each engine to signal “I reached target X”.
  2. A way to hold completion until the orchestrator says “finish now”.

With this, we can hold off the completion of state sync for a set of databases until the full set has converged on the set of targets.

@clabby clabby self-assigned this Mar 20, 2026
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Mar 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 79a8184 Mar 23 2026, 05:43 PM

@clabby clabby requested a review from danlaine March 20, 2026 20:26
@clabby clabby marked this pull request as ready for review March 20, 2026 20:26
Comment thread storage/src/qmdb/sync/engine.rs Outdated
update_receiver: Option<mpsc::Receiver<Target<DB::Digest>>>,

/// Optional receiver for explicit finish requests.
finish_receiver: Option<mpsc::Receiver<()>>,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: I was inconsistent with _rx/_tx vs _receiver/_sender -- should we take this opportunity to make them all _rx/_tx? i.e. change the field names here to match those in the config

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Unified the names on _rx / _tx.

Comment thread storage/src/qmdb/sync/engine.rs Outdated
Comment thread storage/src/qmdb/sync/engine.rs Outdated
@clabby clabby force-pushed the cl/coordinate-sync branch from eb2a35f to e62376c Compare March 23, 2026 14:34
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Mar 23, 2026

Deploying monorepo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 79a8184
Status: ✅  Deploy successful!
Preview URL: https://f200b816.monorepo-eu0.pages.dev
Branch Preview URL: https://cl-coordinate-sync.monorepo-eu0.pages.dev

View logs

Copy link
Copy Markdown
Collaborator

@danlaine danlaine left a comment

Choose a reason for hiding this comment

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

Would you mind adding tests for:

  1. Finish signal arrives before target reached
  2. Caller drops finish_tx without having sent a signal
  3. Caller drops reached_target_rx without having received a signal

Otherwise lgtm

danlaine
danlaine previously approved these changes Mar 23, 2026
Base automatically changed from danlaine/sync-target-update to main March 23, 2026 16:58
@danlaine danlaine dismissed their stale review March 23, 2026 16:58

The base branch was changed.

@clabby clabby force-pushed the cl/coordinate-sync branch from 361ad9a to 79a8184 Compare March 23, 2026 17:42
@clabby clabby enabled auto-merge March 23, 2026 18:10
@clabby clabby added this pull request to the merge queue Mar 23, 2026
Merged via the queue into main with commit c07dd2f Mar 23, 2026
177 of 178 checks passed
@clabby clabby deleted the cl/coordinate-sync branch March 23, 2026 20:10
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 93.75000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.65%. Comparing base (921c288) to head (79a8184).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
storage/src/qmdb/sync/engine.rs 92.22% 5 Missing and 2 partials ⚠️
@@            Coverage Diff             @@
##             main    #3439      +/-   ##
==========================================
- Coverage   95.65%   95.65%   -0.01%     
==========================================
  Files         421      421              
  Lines      148789   148877      +88     
  Branches     3523     3530       +7     
==========================================
+ Hits       142320   142401      +81     
- Misses       5335     5340       +5     
- Partials     1134     1136       +2     
Files with missing lines Coverage Δ
storage/src/qmdb/immutable/sync.rs 98.45% <100.00%> (+0.05%) ⬆️
storage/src/qmdb/sync/error.rs 0.00% <ø> (ø)
storage/src/qmdb/sync/engine.rs 89.60% <92.22%> (+1.25%) ⬆️

... and 7 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 921c288...79a8184. 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

None yet

Development

Successfully merging this pull request may close these issues.

2 participants