Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/skills/monitor-tick/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,7 @@ to overall tick severity — the tick is considered unhealthy when any alert fir
- `recovery_stalled: WARNING absolute=N (post-restart) — investigating` — post-restart absolute fire (absolute ≥ 50 on a baseline-reset tick, #3198)
- `recovery_stalled: skipped (<reason>)` — metric missing or fetch failed
- `recovery_stalled: collecting baseline` — first tick after restart/invalidation (absolute value below the post-restart threshold)
- `recovery_stalled: collecting baseline (cold-catchup exemption)` — a baseline-reset tick whose absolute value ≥ 50 was suppressed because this incarnation completed a from-scratch (HAS-restore) catchup, i.e. a healthy post-wipe recovery, not a stall (`stellar_history_bucket_apply_success_total` > 0, or `FRESH_START=yes`); the warm-restart post-restart fire (#3198) still triggers because a warm restart does no bucket apply (#3816)

**Rendering precedence** (determines the `metrics_ratio:` line format):

Expand Down
57 changes: 55 additions & 2 deletions scripts/lib/eval-alarms.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,14 @@ def make_result(
# only inside extra_values, so without this it never reaches the
# renderer and a post-restart fire is mislabeled (burst) (#3274).
"post_restart": bool(extra_values and extra_values.get("post_restart")),
# Surface the cold-catchup exemption marker (#3816) the same way as
# post_restart above: extra_values keys are NOT auto-promoted, so
# without this the exemption never reaches the renderer and a suppressed
# post-restart fire on a healthy cold catchup renders as a plain
# "collecting baseline" with no visible reason for the quiet.
"cold_catchup_exemption": bool(
extra_values and extra_values.get("cold_catchup_exemption")
),
}

# Safety net: warn about unresolved template placeholders.
Expand Down Expand Up @@ -1482,10 +1490,26 @@ def eval_counter_streak(
start_ticks: str,
gap_stale: bool = False,
too_fresh: bool = False,
fresh_start: bool = False,
) -> dict:
"""Evaluate a counter-streak alarm.

Independent of PREV_PROM_INVALID — uses own PID/start_ticks in snapshot.

Cold-catchup carveout (#3816): the post-restart absolute fire (#3198) exists
to catch a *warm* restart whose startup stall burst straddles the
baseline-reset tick. A *cold* catchup after a state wipe legitimately accrues
>= post_restart_absolute_threshold escalation ticks (being behind for minutes
across ~10^5 ledgers is the point, not a stall). We suppress the one-shot
post-restart fire when this incarnation demonstrably did a from-scratch
HAS-restore catchup — signalled by stellar_history_bucket_apply_success_total
(per-incarnation counter, only emitted on the bucket-apply path, 0/absent on a
warm near-tip restart), OR by FRESH_START=yes as a cheap belt-and-suspenders
arm. A genuinely *stuck* cold catchup is still caught: only the one-shot fire
is exempted; the ordinary same-PID delta/streak/burst path fires on later
ticks if the counter keeps climbing. Keying on bucket-apply (not crash
recovery) is deliberate — #3197's warm-restart stall did NO bucket apply, so
the blind spot the check exists to close stays closed.
"""
ev_default = default_extra_values(alarm, "counter-streak")

Expand All @@ -1512,6 +1536,14 @@ def eval_counter_streak(
return make_result(alarm, "skipped", skip_reason="metric not found",
extra_values=ev_default)

# Cold-catchup signal (#3816), observable at the baseline-reset (fire) tick.
# form1's fallback returns the first series' value even though the counter is
# labeled per-archive; the counter is only exported once it increments (>= 1)
# on a from-scratch HAS-restore catchup, so presence with value > 0 means a
# cold catchup happened this incarnation. Absent (None) on a warm restart.
bucket_apply = extract_value(current, "stellar_history_bucket_apply_success_total", "form1")
cold_catchup = (bucket_apply is not None and bucket_apply > 0) or fresh_start

snapshot_file = alarm.get("snapshot_file", "counter_streak_snapshot")
snapshot_path = state_dir / snapshot_file
snapshot = read_snapshot(snapshot_path)
Expand All @@ -1538,6 +1570,15 @@ def eval_counter_streak(
write_snapshot(snapshot_path, new_snapshot)
post_restart = maybe_post_restart_fire(alarm, cur_val)
if post_restart is not None:
if cold_catchup:
# #3816: a healthy from-scratch catchup legitimately crossed
# the absolute threshold — suppress the one-shot fire. The
# fresh baseline is already written, so a genuinely stuck
# cold catchup still fires via the same-PID delta/streak path.
return make_result(
alarm, "collecting_baseline",
extra_values={"cold_catchup_exemption": True, **ev_default},
)
return post_restart
return make_result(alarm, "collecting_baseline",
extra_values=ev_default)
Expand Down Expand Up @@ -1575,6 +1616,12 @@ def eval_counter_streak(
write_snapshot(snapshot_path, new_snapshot)
post_restart = maybe_post_restart_fire(alarm, cur_val)
if post_restart is not None:
if cold_catchup:
# #3816: cold-catchup exemption, mirroring the PID-change branch.
return make_result(
alarm, "collecting_baseline",
extra_values={"cold_catchup_exemption": True, **ev_default},
)
return post_restart
return make_result(alarm, "collecting_baseline",
extra_values=ev_default)
Expand Down Expand Up @@ -1755,7 +1802,13 @@ def render_aggregate(results: list[dict], watcher_mode: bool) -> dict:
elif r["state"] == "skipped":
recovery_stalled_line = f"recovery_stalled: skipped ({r.get('skip_reason', '')})"
elif r["state"] == "collecting_baseline":
recovery_stalled_line = "recovery_stalled: collecting baseline"
# #3816: surface the cold-catchup exemption so an operator watching a
# state-wipe recovery sees why the post-restart fire stayed quiet,
# instead of an unexplained "collecting baseline".
if r.get("cold_catchup_exemption"):
recovery_stalled_line = "recovery_stalled: collecting baseline (cold-catchup exemption)"
else:
recovery_stalled_line = "recovery_stalled: collecting baseline"
else:
recovery_stalled_line = f"recovery_stalled: ok (delta={r.get('value', 0)})"

Expand Down Expand Up @@ -2089,7 +2142,7 @@ def main() -> int:
)
elif kind == "counter-streak":
result = eval_counter_streak(alarm, current, state_dir, pid, start_ticks_val,
gap_stale, too_fresh)
gap_stale, too_fresh, fresh_start)
else:
result = make_result(alarm, "skipped", skip_reason=f"unknown kind: {kind}")

Expand Down
149 changes: 149 additions & 0 deletions scripts/lib/test_eval_alarms_counter_reset.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,155 @@ def test_post_restart_below_threshold_does_not_fire():
f"Below-threshold reset must not set post_restart, got {result.get('post_restart')!r}"


# ── cold-catchup carveout tests (issue #3816) ────────────────────────────────

def test_post_restart_fire_suppressed_on_cold_catchup():
"""A baseline-reset (PID-change) tick whose absolute counter crosses
post_restart_absolute_threshold must NOT fire post-restart when the node
just completed a from-scratch (HAS-restore) catchup this incarnation —
signalled by stellar_history_bucket_apply_success_total > 0. Being behind
for minutes across ~10^5 ledgers is the point of a cold catchup, not a stall.

Fails on origin/main: eval_counter_streak has no `fresh_start` kwarg
(TypeError) and no cold-catchup gate, so the PID-change branch fires
post_restart at absolute=63.
Passes after: the cold-catchup gate returns collecting_baseline.
"""
with tempfile.TemporaryDirectory() as d:
state_dir = Path(d)
snap_path = state_dir / "counter_streak_snapshot"
write_snapshot(snap_path, {
"version": "1",
"pid": "111",
"start_ticks": "100",
"counter_value": "0",
"breach_streak": "0",
})

alarm = _make_alarm("recovery-stalled", kind="counter-streak",
metric="recovery-stalled-metric",
delta_threshold=1, streak_threshold=3,
burst_threshold=10,
post_restart_absolute_threshold=50,
severity="WARN")
# cur_val 63 >= threshold 50, AND a cold-catchup bucket-apply happened
# this incarnation (labeled per-archive series, value >= 1).
current = {
"recovery-stalled-metric": [({}, 63.0)],
"stellar_history_bucket_apply_success_total": [({"archive": "sdf"}, 1.0)],
}

result = eval_counter_streak(alarm, current, state_dir, "222", "200",
fresh_start=False)

assert result["state"] == "collecting_baseline", \
f"Cold catchup must suppress the post-restart fire, got {result['state']}"
assert not result.get("post_restart"), \
f"Cold catchup must not set post_restart, got {result.get('post_restart')!r}"
assert result.get("cold_catchup_exemption"), \
f"Cold-catchup suppression must set the exemption marker, got {result.get('cold_catchup_exemption')!r}"

# Fresh baseline still written before the (suppressed) fire check.
snap = read_snapshot(snap_path)
assert snap["pid"] == "222", "fresh baseline pid must be written"
assert snap["breach_streak"] == "0", "fresh baseline streak must be 0"


def test_render_cold_catchup_exemption_line():
"""render_aggregate labels a collecting_baseline result carrying the
cold_catchup_exemption marker with the documented exemption suffix.

Fails on origin/main: the renderer has no such branch and emits the plain
`recovery_stalled: collecting baseline` line.
"""
r = {
"contributes_to": "recovery_stalled",
"state": "collecting_baseline",
"cold_catchup_exemption": True,
}
out = render_aggregate([r], watcher_mode=False)
line = out["recovery_stalled_line"]
assert line == "recovery_stalled: collecting baseline (cold-catchup exemption)", \
f"Expected cold-catchup exemption form, got: {line!r}"


def test_post_restart_fire_suppressed_on_fresh_start():
"""The fresh_start OR arm: even without the bucket-apply metric on this tick,
a FRESH_START=yes (state-wipe) tick must suppress the post-restart fire.

Fails on origin/main: no `fresh_start` kwarg (TypeError) and no gate.
"""
with tempfile.TemporaryDirectory() as d:
state_dir = Path(d)
snap_path = state_dir / "counter_streak_snapshot"
write_snapshot(snap_path, {
"version": "1",
"pid": "111",
"start_ticks": "100",
"counter_value": "0",
"breach_streak": "0",
})

alarm = _make_alarm("recovery-stalled", kind="counter-streak",
metric="recovery-stalled-metric",
delta_threshold=1, streak_threshold=3,
burst_threshold=10,
post_restart_absolute_threshold=50,
severity="WARN")
# No bucket-apply series present; fresh_start=True is the only signal.
current = {"recovery-stalled-metric": [({}, 63.0)]}

result = eval_counter_streak(alarm, current, state_dir, "222", "200",
fresh_start=True)

assert result["state"] == "collecting_baseline", \
f"FRESH_START must suppress the post-restart fire, got {result['state']}"
assert not result.get("post_restart"), \
f"FRESH_START must not set post_restart, got {result.get('post_restart')!r}"
assert result.get("cold_catchup_exemption"), \
f"FRESH_START suppression must set the exemption marker, got {result.get('cold_catchup_exemption')!r}"


def test_post_restart_fire_still_fires_warm_restart():
"""Guard for #3197/#3198: a warm near-tip restart (no bucket-apply series,
fresh_start=False) must STILL fire post-restart at absolute >= threshold.
The carveout must be conditional on the cold-catchup signal, not swallow the
warm-restart stall detection the check exists to provide.

Passes before AND after the fix (with the new default kwarg), proving the
carveout is conditional.
"""
with tempfile.TemporaryDirectory() as d:
state_dir = Path(d)
snap_path = state_dir / "counter_streak_snapshot"
write_snapshot(snap_path, {
"version": "1",
"pid": "111",
"start_ticks": "100",
"counter_value": "0",
"breach_streak": "0",
})

alarm = _make_alarm("recovery-stalled", kind="counter-streak",
metric="recovery-stalled-metric",
delta_threshold=1, streak_threshold=3,
burst_threshold=10,
post_restart_absolute_threshold=50,
severity="WARN")
# No bucket-apply series (warm restart, pure ledger-chain replay).
current = {"recovery-stalled-metric": [({}, 63.0)]}

result = eval_counter_streak(alarm, current, state_dir, "222", "200",
fresh_start=False)

assert result["state"] == "firing", \
f"Warm restart must still fire post-restart, got {result['state']}"
assert result.get("post_restart") is True, \
f"Warm restart must set post_restart, got {result.get('post_restart')!r}"
assert not result.get("cold_catchup_exemption"), \
f"Warm restart must not set the exemption marker, got {result.get('cold_catchup_exemption')!r}"


# ── missing-process-identity guard tests (issue #3279) ───────────────────────

def test_missing_identity_does_not_poison_snapshot_or_fire():
Expand Down
Loading