fix(steer): the stall warning re-logged every tick while its own event path deduped - #119
Merged
Merged
Conversation
…t path deduped (AEAB-13)
One undeliverable message to a lane dead six days produced 921 of the 1004 log
lines since the last restart — 92% of the log — and 3570 across the preceding 24
hours. Same lane, same reason, same single queue row, re-stated every tick for 20
hours.
The fix is not a new policy. The `emit_event` calls IMMEDIATELY BELOW this warn
were already deduped on `steer-stalled:{lane}:{cond}:{bucket}` — hourly, per
lane, per condition — and their comment names the exact hazard: "no idem at all
fires every tick, which is the nag AC-310 was filed about." The event path was
fixed; the `tracing::warn!` four lines above it was left behind. Two surfaces
reporting one fact, one deduped and one not.
So the log line now shares that key rather than getting a cadence of its own —
two spellings of "the same stall" would drift, and the drift IS the defect.
`blocked`/`cond`/`bucket` move above the warn; same calls, same frequency, only
the order changed.
In-process rather than DB-backed, deliberately: the event idem is durable because
a missed notification is lost, whereas re-stating an ongoing problem once per
server start is useful rather than noisy.
What it cost, which is why this is worth fixing rather than tolerating: it buried
a first-ever `database is locked` line and two false scheduler warnings during a
log review that existed to find exactly those. Ranking by severity instead of
volume is the only reason they surfaced at all.
The test pins the two properties a careless fix breaks, and BOTH were verified to
fail against a deliberately wrong implementation:
- always-log (the pre-fix behaviour) -> fails assertion 1
- permanent idem (the over-correction) -> fails assertion 4
The second is the important one: an idem that never rolls would fire once and
then hide an ongoing stall forever, which is worse than the bug. It also asserts
that a different CONDITION and a different LANE still log, because collapsing
those is what the surrounding code says made the stall invisible in the first
place.
NOT DONE, and it is a design call rather than a patch: the queue row itself. A
steer aimed at a lane dead six days should arguably be expired or escalated
rather than retried forever. That is the better fix and it makes this one mostly
unnecessary, but it decides someone else's pending message and belongs with the
steer delivery path (LR-21). Recorded on the card.
Verified with CI's exact commands: check --workspace --all-targets clean,
clippy --workspace --all-targets -D warnings exit 0, test --workspace 39 result
lines 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B4vuEScunv4K6RMwQoT9xx
Amux-Session: amux-errors-and-bugs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One undeliverable message to a lane dead six days produced 921 of the 1004 log lines since the last restart — 92% of the log — and 3570 across the preceding 24 hours. Same lane, same reason, same single queue row, re-stated every tick for 20 hours.
This is not a new policy — it's aligning two surfaces that report one fact
The
emit_eventcalls immediately below this warn were already deduped onsteer-stalled:{lane}:{cond}:{bucket}— hourly, per lane, per condition. Their comment names the exact hazard:The event path was fixed. The
tracing::warn!four lines above it was left behind.So the log line now shares that key rather than getting a cadence of its own — two spellings of "the same stall" would drift, and the drift is the defect.
blocked/cond/bucketmove above the warn; same calls, same frequency, only the order changed.In-process rather than DB-backed, deliberately: the event idem is durable because a missed notification is lost, whereas re-stating an ongoing problem once per server start is useful rather than noisy.
What it cost
It buried a first-ever
database is lockedline and two false scheduler warnings during a log review that existed to find exactly those. Ranking by severity instead of volume is the only reason they surfaced.Test
Pins the two properties a careless fix breaks. Both verified to fail against a deliberately wrong implementation:
The second matters more. An idem that never rolls would fire once and then hide an ongoing stall forever — worse than the bug it replaces, and the surrounding code makes exactly this point.
It also asserts a different condition and a different lane still log, because collapsing those is what the surrounding code says made the stall invisible in the first place.
Not done here — a design call, not a patch
The queue row itself. A steer aimed at a lane dead six days should arguably be expired or escalated rather than retried forever. That's the better fix and it makes this one mostly unnecessary — but it decides someone else's pending message and belongs with the steer delivery path (LR-21). Recorded on the card rather than guessed at.
Verification — CI's exact commands
🤖 Generated with Claude Code
https://claude.ai/code/session_01B4vuEScunv4K6RMwQoT9xx