krishvishal commented on code in PR #4073:
URL: https://github.com/apache/iggy/pull/4073#discussion_r3957093120
##########
core/shard/src/lib.rs:
##########
@@ -11247,7 +11416,7 @@ mod view_coverage_tests {
committed_elsewhere: Vec::new(),
};
let held = [sealed(100, 1), sealed(99, 7), sealed(98, 1)];
- let missing = first_op_not_covered(&pending, 0, |op| {
+ let missing = first_op_not_covered(&pending, 0, pending.commit_max,
|op| {
Review Comment:
Fixed. Added
`given_a_held_run_below_the_hole_when_scanning_should_walk_to_the_hole`, which
opens well below the merged commit point and walks a held run to reach the hole.
##########
core/consensus/src/impls.rs:
##########
@@ -1729,6 +1729,33 @@ impl<B: MessageBus, P: Pipeline<Entry = PipelineEntry>>
VsrConsensus<B, P> {
self.recovery_barrier.set(required_commit);
}
+ /// Re-decide the barrier against a log head the cluster just settled.
+ ///
+ /// Boot arms it at the recovered journal head: those ops were acked
before the
+ /// restart, so admitting writes before they re-commit rolls back committed
+ /// history. It otherwise clears only by `commit_max` passing it, which
never
+ /// happens when a view change discards the suffix instead of
re-committing it.
+ /// The boot re-pipeline already ran, so nothing re-prepares those ops,
+ /// `is_caught_up_primary` stays shut, and the primary drops the very
requests
+ /// that would raise `commit_max`.
+ ///
+ /// Call this wherever the head is authoritatively re-decided: a merged
log at
+ /// view start, an adopted `StartView`. `head` lowers the barrier when the
view
+ /// truncated the suffix, keeps it when the suffix survived.
+ ///
+ /// Lowered, never cleared. `is_caught_up_primary` reads it against
`commit_max`
+ /// and a met barrier costs it nothing, but `await_recovery_barrier` reads
it
+ /// against `commit_min`, and adoption raises `commit_max` before walking
the
+ /// suffix into the state machine. Zeroing a met barrier would open that
read
+ /// gate over the unapplied window it exists to hold.
+ pub fn redecide_recovery_barrier(&self, head: u64) {
+ let barrier = self.recovery_barrier.get();
+ if barrier == 0 {
+ return;
+ }
+ self.recovery_barrier.set(barrier.min(head));
Review Comment:
Fixed, both halves. `redecide_recovery_barrier` returns on `head == 0` (zero
is disarmed, not met), and `await_recovery_barrier` re-reads the barrier per
poll instead of capturing it on entry.
##########
core/partitions/src/iggy_partition.rs:
##########
@@ -2081,10 +2081,27 @@ where
.is_some_and(|&high_water| offset <= high_water)
}
- fn apply_consumer_offset_commit(
- &self,
- pending: PendingConsumerOffsetCommit,
- ) -> Result<(), IggyError> {
+ /// Note a committed delete that found no offset to remove.
+ ///
+ /// Expected wherever the paired `AckLevel::NoAck` store never replicated,
so a
+ /// diagnostic and not a fault. Still logged: on a replica that did serve
the
+ /// store it is the first symptom of a lost apply.
+ fn log_absent_offset_delete(&self, kind: &str, id: u64) {
Review Comment:
Not fixed, moot. `log_absent_offset_delete` is gone from HEAD; the rebase
onto master removed that change entirely. Dropped the matching section from the
PR description.
--
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]
