numinnex commented on code in PR #4109:
URL: https://github.com/apache/iggy/pull/4109#discussion_r3982831111
##########
core/consensus/src/dvc_merge.rs:
##########
@@ -221,21 +222,31 @@ fn tally_op<'a>(
// Explicit: the sender proves it never prepared this op.
nacks += 1;
} else if let Some(held) = held {
- // Only a sender BEHIND the canonical log_view can implicitly nack.
- // Without this, corrupting one canonical header in transit turns
every
- // honest sender's correct header into an implicit nack against the
- // garbage: a nack quorum on three replicas. A same-log_view
- // disagreement is evidence, not a vote, and goes through
`conflict`.
- let may_nack_implicitly = dvc.log_view < canonical_log_view;
- match canonical {
- // Implicit: the sender holds a DIFFERENT prepare, so not this
one.
- Some(canonical) if may_nack_implicitly && held.checksum !=
canonical.checksum => {
- nacks += 1;
+ if !offers_body && op > dvc.commit {
+ // A prepare is journaled before it is acked, so a header this
+ // sender cannot serve, above its own commit, proves it is
outside
+ // the ack set. Below that commit a missing body is compaction.
+ nacks += 1;
Review Comment:
The derived nack reads the cached `local_dvc_suffix` snapshot, not the
journal. Its tag is `(op, commit)` only (`impls.rs:2213`), refreshes are gated
on that tag (`shard/lib.rs:10249`, `:10299`), and all five
`invalidate_local_dvc_suffix` sites are removal paths, so nothing invalidates
when a body lands. A backup that adopts a StartView header-only, caches that
shape on a straggler SVC, then appends and acks the retransmitted op still
reports it header-only here, and nacks it.
Two such votes are a nack quorum at n=3, and the `op <= commit_max` refusal
cannot catch it: `head.commit` sits inside `identity_checksum`, pinning
`commit_max <= N-1`. `truncate_from(N)` in `reconcile_metadata_view_divergence`
(`shard/lib.rs:6041-6063`) then erases the acked op from the WAL, logged as
"uncommitted". Pre-PR the same state returned `AwaitingRepair` and kept the
entry.
Suggested fix: fold a journal-mutation generation into the snapshot tag,
bumped on append, truncate and drain, on both planes. A directed 3-replica
probe hits this in 0.09s; a 1400-seed fuzz sweep never finds the ordering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
