krishvishal commented on code in PR #4073:
URL: https://github.com/apache/iggy/pull/4073#discussion_r3957103200
##########
core/shard/src/lib.rs:
##########
@@ -5658,22 +5701,108 @@ where
consensus.group(),
)
.await;
- } else {
- // `from_op` past `to_op` without `commit_min` reaching it: the
- // primary-elect window above starts at the merged log's commit
- // point, which can sit above what this replica has walked. The
- // top-of-tick check closes the ordinary case; this closes the
- // one it cannot see.
- tracing::info!(
+ }
+ }
+ }
+
+ /// Re-arm a repair session that spent its stall budget against another
replica.
+ ///
+ /// A session pins its peer and fences every arming site while it stands,
so a
+ /// peer that cannot answer wedges the walk harder than having no session
at
+ /// all. Past the budget the session is dropped and re-armed one step on;
an
+ /// ordinary lost frame is re-requested long before that. Mirrors the
partition
+ /// rotation in [`Self::tick_partitions`].
+ ///
+ /// Two rings, because two things decide who can serve. A `Normal` backup
is
+ /// repairing its committed tail and any replica ahead of it will do, so it
+ /// walks the cluster preferring the primary. A primary-elect is repairing
+ /// toward a merged log, and only the `DoViewChange` senders that named
the op
+ /// can serve it: walking the whole ring lands on a replica that answers
+ /// `RangeEvicted` for a range it never held.
+ #[allow(clippy::future_not_send)]
+ async fn rotate_stalled_metadata_repair<P>(
+ &self,
+ consensus: &VsrConsensus<B, P>,
+ peer: u8,
+ from_op: u64,
+ to_op: u64,
+ ) where
+ B: MessageBus,
+ P: Pipeline<Entry = consensus::PipelineEntry>,
+ {
+ *self.metadata_repair.borrow_mut() = None;
+ self.metadata_repair_attempts.set(0);
+
+ if consensus.view_log_is_pending() &&
consensus.is_primary_for_view(consensus.view()) {
+ let sources = metadata_view_repair_sources(consensus, from_op);
+ let Some(next_peer) = sources.into_iter().find(|candidate|
*candidate != peer) else {
+ // Only the quiet peer named this op. The session is dropped
either
+ // way: `advance_pending_metadata_view` re-scans on the next
tick and
+ // re-requests it, and a peer that never comes back leaves the
+ // view-change timeout to escalate.
+ tracing::warn!(
shard = self.id,
- to_op,
peer,
- "metadata repair window fully requested; closing the
stalled session"
+ from_op,
+ "no other replica offers op {from_op} for the merged log; \
+ view change is stalled"
);
- *self.metadata_repair.borrow_mut() = None;
- self.note_metadata_repair_progress();
- }
+ return;
+ };
+ let nonce = iggy_common::random_id::get_uuid();
Review Comment:
Fixed. All three sites arm through `arm_metadata_repair_session`, which owns
the nonce / view / `idle_ticks` / `from_op` invariant.
##########
core/consensus/src/impls.rs:
##########
@@ -3668,6 +3698,30 @@ impl<B: MessageBus, P: Pipeline<Entry = PipelineEntry>>
VsrConsensus<B, P> {
sources.into_iter().map(|(_, replica)| replica).collect()
}
+ /// Replicas that committed `op`, most-recent-`log_view` first.
+ ///
+ /// The fallback for an op below the DVC suffixes. A suffix spans
`commit..=op`,
+ /// so [`Self::pending_view_body_sources`] answers nothing about the
committed
+ /// prefix and a merged log whose coverage gap sits there would have no
source
+ /// at all. A sender that committed the op either still journals it or has
+ /// compacted it under a checkpoint, and both answers move the requester
+ /// forward: the prepare, or the `RangeEvicted` that says repair cannot
close
+ /// this gap.
+ ///
+ /// Presence is not proven the way an offered body is, so prefer
+ /// [`Self::pending_view_body_sources`] wherever it returns anything.
+ #[must_use]
+ pub fn pending_view_commit_sources(&self, op: u64) -> Vec<u8> {
Review Comment:
Fixed. Both go through a private `pending_view_sources` taking a `StoredDvc`
predicate.
##########
core/consensus/src/impls.rs:
##########
@@ -4544,6 +4600,32 @@ mod timestamp_clamp_tests {
msg
}
+ /// [`make_start_view`] with commit decoupled from head, for a view that
keeps
+ /// an uncommitted suffix.
+ #[allow(clippy::cast_possible_truncation)]
+ fn make_start_view_with_commit(
Review Comment:
Fixed. `make_start_view` took the `commit` parameter; the duplicate is gone
and its call sites updated.
##########
core/simulator/src/lib.rs:
##########
@@ -1353,6 +1415,42 @@ impl Simulator {
})
}
+ /// The metadata pipeline head the commit walk is holding on, if any. See
+ /// [`CommitPrefixHole`].
+ #[must_use]
+ pub(crate) fn metadata_commit_prefix_hole(
Review Comment:
Fixed. `Simulator::metadata_consensus(idx)` added; the hole readers,
`check_recovery_barrier` and both `state_checker` sites go through it.
--
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]
