krishvishal commented on code in PR #4073:
URL: https://github.com/apache/iggy/pull/4073#discussion_r3957090935
##########
core/shard/src/lib.rs:
##########
@@ -5163,6 +5160,51 @@ where
// `RangeEvicted` again if the primary checkpointed mid
// transfer -- that reraises through the same path, and
each
// round lifts the local floor, so it converges.
+ //
+ // Never as primary-elect. A transfer replaces
snapshot-shaped
+ // state wholesale, and this replica has a merged log
parked
+ // against that state naming ops it has just been told it
+ // cannot serve; installing under it would start the view
over
+ // a log the new state no longer matches. The honest
answer is
+ // that another replica holds the committed prefix, so
leave
+ // the session for the stall rotation to re-target and let
the
+ // view-change timeout escalate if nobody can serve it.
+ if consensus.view_log_is_pending()
+ && consensus.is_primary_for_view(consensus.view())
+ {
+ tracing::warn!(
+ shard = self.id,
+ peer = header.replica,
+ retained_from = header.op,
+ local_commit = consensus.commit_min(),
+ "merged-log repair peer evicted the requested
range; \
+ waiting for another sender rather than
transferring \
+ state mid view change"
+ );
+ return;
+ }
+
+ // The floor must also be ABOVE the op this replica needs.
A
+ // peer behind the requested window walks its serve range
off
+ // the end and answers `RangeEvicted` at the requested
floor
+ // itself, having retained nothing and evicted nothing;
+ // converting on that arms a transfer against a replica
with
+ // less state than this one and fences repair for a full
+ // transfer backoff. Drop the session and let the level
trigger
+ // re-request from the primary instead.
+ if header.op <= consensus.commit_min() + 1 {
+ tracing::warn!(
+ shard = self.id,
+ peer = header.replica,
+ retained_from = header.op,
+ local_commit = consensus.commit_min(),
+ "metadata repair peer retains the requested floor
but served \
+ nothing; re-requesting rather than converting to
state transfer"
+ );
+ *self.metadata_repair.borrow_mut() = None;
+ self.metadata_repair_attempts.set(0);
Review Comment:
Fixed. The arm charges a round instead of clearing it, so the state-transfer
escalation stays reachable. Log text now matches the comment.
##########
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);
Review Comment:
Fixed. Rotation no longer touches the budget. Only
`note_metadata_repair_walked` clears it, and the field doc says so.
##########
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 {
Review Comment:
Fixed. `next_view_repair_peer` walks the list as a wrapping ring from
`avoid`, and rotation resolves sources for `session.from_op`, the op the scan
actually reported. Ring tests added.
##########
core/shard/src/lib.rs:
##########
@@ -9926,6 +10059,37 @@ where
}
}
+/// Lowest op of a primary-elect's merged log this replica can be held to.
+///
+/// The merged commit point is what the cluster committed, `commit_min` what
this
+/// replica applied; they diverge exactly when the local prefix has a hole. The
Review Comment:
Fixed. Doc now says they diverge whenever the merged commit point is
unapplied: hole or plain apply lag.
##########
core/shard/src/lib.rs:
##########
@@ -10630,9 +10794,14 @@ const fn header_is_view_entry(local: &PrepareHeader,
canonical: &PrepareHeader)
/// Neither can diverge from the merged log (a committed or compacted op is the
/// quorum's op), and no repair puts the journal entry back, so demanding one
parks
/// the view change forever.
+///
+/// Opens at [`merged_log_scan_floor`]: the merged commit point alone would
declare
+/// the log serveable over a local gap, promoting a replica whose
`CommitJournal`
+/// gap-stops below where `RebuildPipeline` seeds.
fn first_op_not_covered(
pending: &MergedLog,
repair_floor: u64,
+ commit_min: u64,
Review Comment:
Fixed. `first_op_not_covered` takes a named `ScanFloor { repair_floor,
commit_min }`.
--
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]
