Skip to content

fix(shard): widen partition repair when a view-adopted suffix is missing bodies - #4074

Open
mattp5657 wants to merge 2 commits into
apache:masterfrom
mattp5657:fix/cluster-commit-stalls-after-view-change
Open

fix(shard): widen partition repair when a view-adopted suffix is missing bodies#4074
mattp5657 wants to merge 2 commits into
apache:masterfrom
mattp5657:fix/cluster-commit-stalls-after-view-change

Conversation

@mattp5657

@mattp5657 mattp5657 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Closes

#4075

Rationale

A partition group can wedge permanently and stop committing after a view change lands while a node is mid-restart, hanging every producer to that partition until its read timeout expires.

What changed?

After a view change, a backup that adopts a StartView suffix advances its sequencer to the new head immediately, but repair only fetched bodies up to the commit point when the replica also had a commit lag, which a just-restarted node always does. The adopted op's body was never fetched. Once any later op advanced the sequencer past it, the backup's replication gap check rejected every retransmission of the missing op as a stale duplicate, permanently, and commit could never advance past it.

Widens the repair fetch window using missing_suffix, already computed for this but not consulted in this branch, so the first repair pass covers the adopted suffix before anything else can move the sequencer past it.

Fixes cluster::fast_primary_rejoin::given_a_quic_producer_when_its_first_roster_hop_is_down_should_reach_the_partition_primary, failing ~70-90% of local runs and on all 4 CI attempts.

Local Execution

  • Passed. Fixed test: 12/12 runs. WebSocket sibling: 5/5. Full fast_primary_rejoin.rs (8 tests), full cluster:: integration suite (61 tests), and consensus/partitions/shard unit suites (462 tests): all pass, no regressions.
  • Pre-commit hooks: not run through this session; cargo fmt, cargo sort --no-format, and cargo clippy were run manually.

AI Usage

Claude was used in the generation of this.

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 44.84%. Comparing base (7b78014) to head (3408b8f).

Files with missing lines Patch % Lines
core/shard/src/lib.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #4074       +/-   ##
=============================================
- Coverage     85.55%   44.84%   -40.71%     
  Complexity     1402     1402               
=============================================
  Files          1241     1240        -1     
  Lines        186306   155199    -31107     
  Branches     152594   121489    -31105     
=============================================
- Hits         159395    69605    -89790     
- Misses        22865    83746    +60881     
+ Partials       4046     1848     -2198     
Components Coverage Δ
Rust Core 35.32% <66.66%> (-51.14%) ⬇️
Java SDK 67.29% <ø> (ø)
C# SDK 56.94% <ø> (-20.09%) ⬇️
Python SDK 90.06% <ø> (ø)
PHP SDK 85.65% <ø> (ø)
Node SDK 94.50% <ø> (-1.74%) ⬇️
Go SDK 69.35% <ø> (+0.03%) ⬆️
Files with missing lines Coverage Δ
core/shard/src/lib.rs 69.46% <66.66%> (-12.25%) ⬇️

... and 653 files with indirect coverage changes

🚀 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.

@mattp5657
mattp5657 marked this pull request as ready for review September 6, 2026 14:04
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Sep 6, 2026

@ethanlin01x ethanlin01x left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread core/shard/src/lib.rs
// missing bodies sit above `commit_to_op`, not within it -- so the
// cap only holds when no suffix is missing; otherwise it must widen
// to `head` to ever reach those bodies.
let fetch_to_op = if commit_lag && !missing_suffix {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Could you add a unit test for the commit_lag && missing_suffix case?

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

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants