spetz commented on code in PR #4506:
URL: https://github.com/apache/iggy/pull/4506#discussion_r4269769204
##########
core/consensus/src/plane_helpers.rs:
##########
@@ -903,6 +903,84 @@ where
.await
}
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum ViewLogProof {
+ Proven,
+ /// Fetch `op`; retained entries above `unproven_through` match the view.
+ Missing {
+ op: u64,
+ unproven_through: u64,
+ },
+ Divergent(u64),
+}
+
+/// Prove held entries against the view's hash chain. Canonical headers can
+/// bridge missing bodies; a gap with no header leaves its lower prefix
unknown.
+#[must_use]
+pub fn view_log_proof(
+ commit_floor: u64,
+ canonical: &[PrepareHeader],
+ op_head: u64,
+ header_at: impl Fn(u64) -> Option<PrepareHeader>,
+) -> ViewLogProof {
+ let proven = locally_proven_commit(commit_floor, &header_at);
+ if op_head <= proven {
+ return ViewLogProof::Proven;
+ }
+ let Some(head) = canonical.first().filter(|head| head.op == op_head) else {
+ return ViewLogProof::Missing {
+ op: op_head,
+ unproven_through: op_head,
+ };
+ };
+ let mut expected = head.checksum;
+ let mut missing = None;
+ for op in (proven + 1..=op_head).rev() {
+ let header = if let Some(local) = header_at(op) {
+ local
+ } else if let Some(header) = canonical.iter().find(|header| header.op
== op) {
+ missing.get_or_insert(op);
+ *header
+ } else {
+ return ViewLogProof::Missing {
+ op,
+ unproven_through: op,
+ };
+ };
+ if header.checksum == CHECKSUM_UNSEALED || header.checksum != expected
{
Review Comment:
`view_log_proof` reads a blank canonical slot (`dvc_blank`, checksum 0,
which equals `CHECKSUM_UNSEALED`) as `Divergent(op)`, while `view_header_at`
and `header_is_view_entry` treat the same slot as no evidence.
`build_dvc_suffix` writes that blank at the commit point when the sender holds
no header there (`core/shard/src/lib.rs:11553-11570`, whose warning already
names this state), and both the probe answer
(`core/consensus/src/impls.rs:3489-3500`) and the stale-heartbeat answer
(`core/consensus/src/impls.rs:3805`) ship that suffix. A replica that installs
a metadata snapshot at op S on an idle cluster and becomes primary before S+1
commits answers every probe with `[dvc_blank(S)]`, and a prober that lacks S (a
node rebuilt with an empty data directory, or a second lagging backup with 5
replicas) gets `Divergent(S)`. When the blank is the head, line 936 seeds
`expected` with 0, so even a correct local header reads as divergent. On the
metadata plane `reconcile_metadata_view_diver
gence` then calls `remove_range(S..=S)` at `core/shard/src/lib.rs:6487`, which
returns `Ok(0)` before its only await
(`core/journal/src/prepare_journal.rs:997`), and `pending` is cloned once, so
the loop never yields and freezes the shard thread at 100% CPU. A restart does
not help until the primary commits a later op. On the partition plane every
`ViewApplied` re-defers the same one-op truncation, and the replica stays
unproven and sends no PrepareOk. Unit tests at this head reproduce all three:
the proof returns `Divergent(4)` for an op the backup does not hold, the
metadata reconcile does not finish within 20s while the same scenario with an
empty suffix passes in 0.02s, and the partition truncation is re-deferred 256
times. Both repair ingest gates (`core/shard/src/lib.rs:5521` and
`core/partitions/src/iggy_partition.rs:11650`) also compare repaired prepares
against the blank checksum 0 and drop every copy.
--
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]
