Repository navigation
fix(pipeline): fail fast on a wedged pipeline instead of hanging forever - #407
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (88.93%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## feat-runall #407 +/- ##
==============================================
Coverage ? 93.27%
==============================================
Files ? 138
Lines ? 54910
Branches ? 0
==============================================
Hits ? 51220
Misses ? 3690
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The typed-step deadlock monitor only logged a stall warning each window and
never failed the run, so a genuine wedge (no worker can make progress) hangs
forever until killed externally — a regression from main, where the watchdog
records a TimedOut error and tears the pipeline down. The reorder-stash OOM fix
earlier in this stack also removed the one accidental backstop (unbounded
growth → OOM-kill), so a wedge now hangs cleanly with flat, bounded memory.
Restore main's guarantee with its starvation guard. On a stall, sum the bytes
in flight across transport queues AND reorder overflow stashes (the latter is
essential — items can sit in a reorder buffer waiting for a missing serial
while every transport reads empty):
- nothing in flight → idle, waiting on a slow upstream (e.g. a stdin pipe);
not a deadlock — reset the clock and keep watching (mirrors main's
`check_deadlock_and_restore`);
- work stuck past the warn window → warn with a stats snapshot, once per
window;
- work stuck past the fatal window → record `PipelineError::TimedOut` and
`cancel()`, so workers observe `is_done()` and the run fails fast.
Diverge from main's single 10s kill: a single huge dispatch (busy-locus group,
large sort merge) can legitimately flatten progress for many seconds with work
queued, so escalate to fatal only after the stall persists 6× the warn window
(~60s by default at `--deadlock-timeout 10`). A real deadlock hangs forever, so
waiting longer to be sure is free, whereas a false kill on a slow workload would
be a regression. `--deadlock-timeout 0` disables the monitor entirely.
The stall classifier (`classify_stall`) and the in-flight probe
(`in_flight_bytes`) are extracted as pure functions and unit-tested across the
progressing / starving / watching / stalled / wedged cases.
nh13
force-pushed
the
feat-runall-15-wedged-pipeline-watchdog
branch
from
June 15, 2026 17:41
8a70320 to
deb6314
Compare
nh13
added a commit
that referenced
this pull request
Jun 18, 2026
…ver (#407) The typed-step deadlock monitor only logged a stall warning each window and never failed the run, so a genuine wedge (no worker can make progress) hangs forever until killed externally — a regression from main, where the watchdog records a TimedOut error and tears the pipeline down. The reorder-stash OOM fix earlier in this stack also removed the one accidental backstop (unbounded growth → OOM-kill), so a wedge now hangs cleanly with flat, bounded memory. Restore main's guarantee with its starvation guard. On a stall, sum the bytes in flight across transport queues AND reorder overflow stashes (the latter is essential — items can sit in a reorder buffer waiting for a missing serial while every transport reads empty): - nothing in flight → idle, waiting on a slow upstream (e.g. a stdin pipe); not a deadlock — reset the clock and keep watching (mirrors main's `check_deadlock_and_restore`); - work stuck past the warn window → warn with a stats snapshot, once per window; - work stuck past the fatal window → record `PipelineError::TimedOut` and `cancel()`, so workers observe `is_done()` and the run fails fast. Diverge from main's single 10s kill: a single huge dispatch (busy-locus group, large sort merge) can legitimately flatten progress for many seconds with work queued, so escalate to fatal only after the stall persists 6× the warn window (~60s by default at `--deadlock-timeout 10`). A real deadlock hangs forever, so waiting longer to be sure is free, whereas a false kill on a slow workload would be a regression. `--deadlock-timeout 0` disables the monitor entirely. The stall classifier (`classify_stall`) and the in-flight probe (`in_flight_bytes`) are extracted as pure functions and unit-tested across the progressing / starving / watching / stalled / wedged cases.
This branch was previously deployed
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.
Summary
The typed-step deadlock monitor only logged a stall warning each window and never failed the run, so a genuine wedge (no worker can make progress) hung forever until killed externally — a regression from the prior behaviour, where the watchdog records a
TimedOuterror and tears the pipeline down. (The reorder-stash OOM fix also removed the one accidental backstop — unbounded growth → OOM-kill — so a wedge now hangs cleanly with flat, bounded memory.)Fix
Restore the fail-fast guarantee with a starvation guard. On a stall, sum the bytes in flight across the byte-bounded transport queues and their reorder overflow stashes (the stash is essential — items can sit in a reorder buffer waiting for a missing serial while every transport reads empty), then classify:
PipelineError::TimedOutandcancel(), so workers observeis_done()and the run fails fast.Escalate to fatal only after the stall persists 6× the warn window (~60s by default at
--deadlock-timeout 10): a single huge dispatch (busy-locus group, large sort merge) can legitimately flatten progress for many seconds with work queued, and a real deadlock hangs forever, so waiting longer to be sure is free whereas a false kill on a slow workload would be a regression.--deadlock-timeout 0disables the monitor entirely.classify_stalland the in-flight probein_flight_bytesare extracted as pure functions and unit-tested across the progressing / starving / watching / stalled / wedged cases.Known limitation (documented in code)
The in-flight probe covers byte-bounded transport edges only;
CountBounded/Unboundedtransports are not registered as queue handles, so they don't contribute. The only currentCountBoundeduser is the sort spill→merge path, whoseSortMergeis deadlock-free by construction (the cooperative slot-refill that fixed #330 BUG #3), so a wedge there is not reachable and the monitor correctly stays quiet during a slow merge. Extending the probe to count-based queues for defence-in-depth is filed as #408.Stack context
Follow-up fix for the issue #330 unified-pipeline landing onto
feat-runall(one commit per PR). Dual-reviewed before push (CLI: no findings; skill: traced the teardown/error-propagation, false-positive guard, andclassify_stallboundaries — sound; surfaced the byte-bounded-only probe scope, now documented + filed as #408).