You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by kr...@apache.org on 2026/09/15 07:54:36 UTC

(iggy) 03/03: fix(consensus): carry the suffix on the last bare StartView too

This is an automated email from the ASF dual-hosted git repository.

krishvishal pushed a commit to branch simulator-liveness
in repository https://gitbox.apache.org/repos/asf/iggy.git

commit b7f221787f744e8b7cea2f2f737af6a5e8342454
Author: Krishna Vishal <kr...@gmail.com>
AuthorDate: Thu Sep 10 12:34:43 2026 +0530

    fix(consensus): carry the suffix on the last bare StartView too
    
    Three sites emit a StartView and only the view-change completion carried
    the view's headers. The probe answer was the first to be corrected; the
    stale-view heartbeat answer is the last, and it had the same two defects
    for the same stated reason: it published a frontier and left the peer to
    reach it by repair.
    
    Repair skips an op whose header is already resident, so a peer holding a
    different entry at an op under the announced commit point never learns of
    it that way. It adopts the commit point and applies what it already has.
    The announced commit was also unclamped, which StartViewHeader::validate
    rejects as commit > op and the dispatcher panics on.
    
    Unmeasured: the path answers a heartbeat from a peer stuck on an older
    view, which fires once across 7000 runs, and the failure set is
    byte-identical either way. Landed because leaving one of three emitters
    wrong is how the defect comes back, not because a seed closed.
    
    dvc_commit is public now, since two planes and the shard all need the one
    definition of the announceable commit point. Its doc had a stale copy of
    handle_do_view_change's above it, which making it public would have
    published.
---
 core/consensus/src/impls.rs | 12 ++++--------
 core/shard/src/lib.rs       | 10 ++++++----
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/core/consensus/src/impls.rs b/core/consensus/src/impls.rs
index d9d366779..3ee527312 100644
--- a/core/consensus/src/impls.rs
+++ b/core/consensus/src/impls.rs
@@ -2947,13 +2947,8 @@ impl<B: MessageBus, P: Pipeline<Entry = PipelineEntry>> VsrConsensus<B, P> {
         actions
     }
 
-    /// Handle a received `DoViewChange` message (only relevant for primary candidate).
-    ///
-    /// "When the new primary receives f + 1 DOVIEWCHANGE messages from different
-    /// replicas (including itself), it sets its view-number to that in the messages
-    /// and selects as the new log the one contained in the message with the largest v'..."
-    ///
-    /// The `commit` this replica advertises in a `DoViewChange`.
+    /// The `commit` this replica advertises in a `DoViewChange`, and in any
+    /// `StartView` it announces.
     ///
     /// `commit_max`, not `commit_min`: the new primary floors its pipeline rebuild
     /// at `max(commit)` across the quorum, and only `commit_max` bounds that range
@@ -2964,7 +2959,8 @@ impl<B: MessageBus, P: Pipeline<Entry = PipelineEntry>> VsrConsensus<B, P> {
     /// the prepares and `DoViewChangeHeader::validate` rejects `commit > op`.
     /// Lossless for the rebuild floor: quorum intersection guarantees some sender
     /// whose head covers the true commit point carries it.
-    fn dvc_commit(&self) -> u64 {
+    #[must_use]
+    pub fn dvc_commit(&self) -> u64 {
         let op = self.sequencer.current_sequence();
         self.commit_max.get().min(op)
     }
diff --git a/core/shard/src/lib.rs b/core/shard/src/lib.rs
index 27d15c7a6..0187a70dc 100644
--- a/core/shard/src/lib.rs
+++ b/core/shard/src/lib.rs
@@ -10378,13 +10378,15 @@ where
     let action = VsrAction::SendStartView {
         view: consensus.view(),
         op: consensus.sequencer().current_sequence(),
-        commit: consensus.commit_max(),
+        commit: consensus.dvc_commit(),
         incarnation: 0,
         target: None,
         group: consensus.group(),
-        // Correcting a peer on a stale view, not concluding a view change: this
-        // publishes the settled frontier, which the peer reaches by repair.
-        suffix: Vec::new(),
+        // The headers, not just the frontier. Repair skips an op whose header is
+        // already resident, so a peer holding a DIFFERENT entry at an op under
+        // this commit point never learns of it from repair alone: it adopts the
+        // commit point and applies what it already has.
+        suffix: consensus.local_dvc_suffix().headers().to_vec(),
     };
     dispatch_vsr_actions::<B, P, J>(consensus, None, &[action]).await;
 }