Dump the compile traceback before the runner kills the process - #1518
Merged
Conversation
The watchdog shipped in #1517 never actually dumped. Its CI default was a flat 300s and the workspace smoke cap is also 300s, so the dump timer and the runner's kill timer raced and the kill won -- a SIGKILLed process writes no traceback. Measured: 20 stalled runs across four CI legs produced heartbeats from every one of them and not a single stack (autolens_workspace_test#271). Derive the CI default from BUILD_SCRIPT_TIMEOUT -- the per-script cap the workspace runners and PyAutoHands both enforce -- at 80% of it, so the traceback has time to reach stderr before the kill lands. 300s cap dumps at 240s; 1800s cap dumps at 1440s. With no usable cap advertised, fall back to a flat 240s rather than computing a fraction of zero, which would silently disable the dump. An explicit PYAUTOFIT_JAX_COMPILE_DUMP_SECS still wins. Four new tests, one of them pinning the invariant directly: the threshold is strictly below the cap for every cap. Verified end-to-end: with a 10s cap the dump fires at 8s and the traceback is captured, where before nothing was written at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qk7hoavMnFyPtW4toYn8K
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.
Fixes a defect in #1517, found by the measurement #1517 existed to enable. Part of the
jax-compile-stallepic; evidence in autolens_workspace_test#271.The bug
The
faulthandlerwatchdog shipped in #1517 never fired. Its CI default was a flat300.0seconds — and the workspace smoke cap is also 300s. The dump timer and the runner's kill timer therefore raced, and the kill won every time. A SIGKILLed process writes no traceback.This is not theoretical. The first CI use of the watchdog was a re-timing sweep over four quarantined JAX scripts, 5 repeats × 2 Python versions across two repos. 20 of those runs stalled to the cap. All 20 produced heartbeats. None produced a stack:
So #1517 delivered the half that proves a stalled run is alive, and silently dropped the half that says where it is stuck — which is the half phase 3 of the epic actually needs.
The fix
Derive the CI default from
BUILD_SCRIPT_TIMEOUT, the per-script cap that the workspace runners and PyAutoHands both enforce, at 80% of it:BUILD_SCRIPT_TIMEOUT300(smoke)1800(release)0/ malformedThe fallback matters: computing a fraction of an absent cap would yield
0, which disables the dump — the same silent no-op in a new disguise. An explicitPYAUTOFIT_JAX_COMPILE_DUMP_SECSstill overrides everything.Verification
Full suite 2011 passed, 34 skipped.
test_jax_compile.py19 passed, 4 new — including one that pins the invariant directly rather than testing today's numbers:End-to-end, a simulated stall with a 10s cap — the dump now lands at 8s, before the kill at 12s:
Same scenario before this change wrote no traceback at all.
Note on the original review
Nothing in #1517's tests could have caught this. They monkeypatched
faulthandlerand asserted the timer was armed and cancelled — which it was, correctly. The failure lives in the relationship between two independently-correct timeouts owned by different repos, and only a real run under a real cap exposes it. Hence the new test asserting the relationship rather than the value.Generated by Claude Code