Skip to content

fix(consensus): count an unservable header as a nack - #4109

Merged
spetz merged 4 commits into
masterfrom
consensus-dvc-derived-nack
Sep 11, 2026
Merged

spetz merged 4 commits into
masterfrom
consensus-dvc-derived-nack

Conversation

@krishvishal

Copy link
Copy Markdown
Member

A view change can reach a state where an op is neither recoverable from the DoViewChange messages in hand nor provably uncommitted. merge_dvc_quorum answers AwaitingRepair and waits for every replica to report, so a permanently crashed replica leaves two healthy survivors unable to elect a primary. The view number keeps climbing while log_view stays put, and client requests retry forever.

The op was decidable from the quorum already present. A sender that holds the header but cannot serve the body counted as neither a copy nor a nack. A prepare is journaled before it is acked and nothing compacts an op above the commit point, so a missing body proves that sender never acked it, which is what a nack asserts.

tally_op now derives a nack from that sender. The quorum is unchanged at replica_count - quorum_replication + 1, so replicas provably outside the ack set still leave fewer than a replication quorum inside it, and a sender that can serve the body is untouched. The derived nack stops at the sender's own commit point, where a missing body means compaction rather than absence.

Two unit tests now assert truncation where they asserted a stall, one pins the boundary at a single servable copy, and the undecidable case moves to abstention. A simulator test replays the seed that found this: without the change the cluster reaches view 103 against log_view 3 with both survivors caught up, and one request retries 266 times unanswered.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Sep 9, 2026
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.80952% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.18%. Comparing base (f87ef77) to head (afcd4a9).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
core/consensus/src/dvc_merge.rs 86.84% 5 Missing ⚠️
core/partitions/src/iggy_partition.rs 63.63% 0 Missing and 4 partials ⚠️
core/consensus/src/plane_helpers.rs 98.86% 0 Missing and 1 partial ⚠️
core/metadata/src/impls/metadata.rs 80.00% 0 Missing and 1 partial ⚠️
core/shard/src/lib.rs 66.66% 1 Missing ⚠️
core/simulator/src/lib.rs 97.95% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #4109       +/-   ##
=============================================
- Coverage     86.38%   72.18%   -14.21%     
+ Complexity     1455     1454        -1     
=============================================
  Files          1259     1257        -2     
  Lines        205513   183289    -22224     
  Branches     170714   148511    -22203     
=============================================
- Hits         177542   132301    -45241     
- Misses        23541    46631    +23090     
+ Partials       4430     4357       -73     
Components Coverage Δ
Rust Core 69.67% <93.80%> (-17.67%) ⬇️
Java SDK 67.57% <ø> (-0.02%) ⬇️
C# SDK 76.97% <ø> (ø)
Python SDK 91.34% <ø> (ø)
PHP SDK 85.65% <ø> (-0.01%) ⬇️
Node SDK 96.15% <ø> (-0.03%) ⬇️
Go SDK 69.43% <ø> (+0.03%) ⬆️
Files with missing lines Coverage Δ
core/consensus/src/impls.rs 93.06% <100.00%> (+0.13%) ⬆️
core/consensus/src/plane_helpers.rs 95.30% <98.86%> (-0.13%) ⬇️
core/metadata/src/impls/metadata.rs 88.88% <80.00%> (-0.31%) ⬇️
core/shard/src/lib.rs 85.13% <66.66%> (+0.37%) ⬆️
core/simulator/src/lib.rs 96.35% <97.95%> (+0.01%) ⬆️
core/partitions/src/iggy_partition.rs 92.30% <63.63%> (-0.04%) ⬇️
core/consensus/src/dvc_merge.rs 93.33% <86.84%> (-0.29%) ⬇️

... and 325 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.

Comment thread core/consensus/src/dvc_merge.rs
@krishvishal
krishvishal force-pushed the consensus-dvc-derived-nack branch from 0aa8560 to f310fd0 Compare September 11, 2026 12:38
@spetz
spetz merged commit 73a0bed into master Sep 11, 2026
100 checks passed
@spetz
spetz deleted the consensus-dvc-derived-nack branch September 11, 2026 12:57
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Sep 11, 2026
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.

3 participants