krishvishal commented on code in PR #4073:
URL: https://github.com/apache/iggy/pull/4073#discussion_r3957088584
##########
core/consensus/src/plane_helpers.rs:
##########
@@ -482,15 +510,33 @@ where
/// revalidates that the head is still this exact entry before popping and
/// applying it. A driver dropped at an await strands nothing; a sibling driver
/// that committed the op first fails the caller's revalidation and re-peeks.
+///
+/// Bounded below for the reason [`drain_committable_prefix`] is, and reported
+/// rather than asserted for the same one. Holding is safe: a shard pump's
panic is
+/// swallowed by `compio::runtime::spawn`, while `tick_metadata` re-arms
repair on
+/// the level.
pub fn peek_committable_head<B, P>(consensus: &VsrConsensus<B, P>) ->
Option<PrepareHeader>
where
B: MessageBus,
P: Pipeline<Entry = PipelineEntry>,
{
let commit = consensus.commit_max();
- consensus
+ let next = consensus.commit_min() + 1;
+ let head = consensus
.pipeline_head_header()
- .filter(|header| header.op <= commit)
+ .filter(|header| header.op <= commit)?;
+ if head.op != next {
Review Comment:
Fixed by the same cap. `peek_committable_head` can no longer see a head
under its floor, so no reply and no `reply_sender` is stranded.
##########
core/consensus/src/plane_helpers.rs:
##########
@@ -435,8 +435,18 @@ where
/// Drain and return committable prepares from the pipeline head.
///
-/// Entries are drained only from the head and only while their op is covered
-/// by the current commit frontier.
+/// Entries are drained from the head, while covered by the commit frontier,
and
+/// only as a contiguous run starting at the next op owed to the state machine.
+///
+/// Callers `advance_commit_min` per entry, so a run starting above
+/// `commit_min + 1` or breaking partway hits that counter's sequential-advance
+/// assert. Pipeline-side twin of `commit_journal`'s gap-stop.
+///
+/// Reported, not asserted: a promoted partition primary reaches it
legitimately,
+/// `RebuildPipeline` seeding above `commit_min` while the walk that catches
it up
+/// runs `COMMIT_WALK_OPS_MAX` ops per call. `commit_journal`'s journal
fallback
Review Comment:
Not fixed. Premise confirmed false and the docs no longer rest on it;
`committed_headers_from` is still ring-blind against an eviction reaching
`commit_max`, flagged as a known gap at `collect_committable_from_journal`. A
ring read is not a one-line swap: the apply path needs batch bytes and the ring
is capacity-bounded.
##########
core/consensus/src/plane_helpers.rs:
##########
@@ -446,18 +456,35 @@ where
P: Pipeline<Entry = PipelineEntry>,
{
let commit = consensus.commit_max();
+ let commit_min = consensus.commit_min();
+ let replica = consensus.replica();
let mut drained = Vec::new();
consensus.with_pipeline_mut(|pipeline| {
+ let mut next = commit_min + 1;
while let Some(head_op) = pipeline.head().map(|entry| entry.header.op)
{
if head_op > commit {
break;
}
+ if head_op != next {
+ tracing::warn!(
Review Comment:
Fixed. Both gates route through `report_uncommittable_head` at `debug!`,
matching `tick_partitions`/`tick_metadata`, with a different message per arm: a
hole waits on repair, an applied head is an `error!` defect.
##########
core/shard/src/lib.rs:
##########
@@ -5499,9 +5541,12 @@ where
// from the evicted ring or the flushed segments.
let missing = {
let journal = partition.log.journal();
- first_op_not_covered(&pending, consensus.commit_min(), |op| {
- journal.inner.header_by_op(op)
- })
+ first_op_not_covered(
+ &pending,
+ consensus.commit_min(),
+ consensus.commit_min(),
Review Comment:
Fixed. Scan probes ring+resident in one `repair_headers_in` pass, and a
genuine gap now arms `request_partition_view_repair`. The tick keeps that
session live outside `Normal`.
##########
core/shard/src/lib.rs:
##########
@@ -4972,6 +4975,9 @@ where
if !in_scope {
return;
}
+ // The serving peer is answering. Clears the stall clock and the
budget
+ // so a window served in chunks cannot rotate off a healthy stream.
+ self.note_metadata_repair_progress();
Review Comment:
Fixed. Restored to master's position: below `disagrees` and
`verify_prepare_integrity`, above the dedup return.
--
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]
