[None][test] Dump a stack traceback when a test hangs#16374
Conversation
068ad07 to
37412ea
Compare
📝 WalkthroughWalkthroughPeriodic pytest reporting gains an opt-in ChangesPeriodic hang traceback reporting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Pytest
participant PeriodicJUnitXML
participant faulthandler
participant TracebackFile
Pytest->>PeriodicJUnitXML: configure hang traceback
PeriodicJUnitXML->>faulthandler: enable output
Pytest->>PeriodicJUnitXML: start test
PeriodicJUnitXML->>faulthandler: schedule timeout dump
faulthandler->>TracebackFile: write thread stacks
Pytest->>PeriodicJUnitXML: finish test
PeriodicJUnitXML->>faulthandler: cancel watchdog
Pytest->>PeriodicJUnitXML: signal shutdown
PeriodicJUnitXML->>faulthandler: dump all thread stacks
faulthandler->>TracebackFile: flush output
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/defs/conftest.py`:
- Around line 2086-2095: Add regression tests in test_conftest.py and
test_periodic_junit.py covering the default and explicitly enabled
--periodic-hang-traceback values, propagation into PeriodicJUnitXML via
dump_hang_traceback, and the related registration log. Keep these tests narrowly
scoped and independent of GPU or model access, using existing test fixtures and
symbols where available.
In `@tests/integration/defs/utils/periodic_junit.py`:
- Around line 108-112: Annotate the new hang-traceback state and all related
helpers and pytest hooks in the surrounding changes, including _hang_file, with
precise types and explicit return types. Type each pytest item parameter using
the appropriate item type, avoid Any, and ensure annotations cover the functions
through the referenced range while preserving their existing behavior.
- Around line 433-442: The SIGINT/SIGTERM traceback dump currently depends on
the Python handler running, so native C/CUDA hangs may prevent it. In the
setup/registration flow associated with the existing hang handler and _hang_file
persistence, register faulthandler for SIGINT and SIGTERM with chain=True while
retaining the current handler for report persistence; extend the integration
tests to cover signal delivery in subprocesses.
- Around line 376-421: Update _effective_timeout to match pytest-timeout
resolution, including keyword timeout marker values, global timeout fallback,
and func_only semantics. Move watchdog cancellation from pytest_runtest_teardown
to the teardown phase that runs after fixture finalizers, preserving the
existing arming behavior and warning handling. Add coverage in
test_periodic_junit.py for keyword/global resolution, func_only markers, and a
hanging fixture finalizer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0adafb3d-bb2e-410b-be31-cd3834c7f715
📒 Files selected for processing (3)
jenkins/L0_Test.groovytests/integration/defs/conftest.pytests/integration/defs/utils/periodic_junit.py
37412ea to
3c4c7fb
Compare
|
/bot run |
|
PR_Github #59204 [ run ] triggered by Bot. Commit: |
|
PR_Github #59204 [ run ] completed with state
|
3c4c7fb to
cc32cf7
Compare
|
/bot run |
|
|
|
/bot run |
|
PR_Github #59339 [ run ] triggered by Bot. Commit: |
|
PR_Github #59339 [ run ] completed with state
|
A per-test timeout or stage wall-clock kill currently leaves only an empty "Test terminated unexpectedly" record, so a hang is named (via unfinished_test.txt) but not diagnosable — there is no stack showing where it is stuck. Add opt-in hang-traceback dumping to the periodic-junit plugin: - Watchdog (primary): arm faulthandler.dump_traceback_later() at each test's setup for ~90% of its timeout and cancel on teardown, so every thread's stack is written to output-dir/hang_traceback.txt just before the test is killed. The watchdog thread fires even when the main thread is stuck in an un-interruptible C/CUDA call — exactly when the thread-method per-test timeout cannot unwind it. - Signal path: also dump on SIGINT/SIGTERM (e.g. a stage wall-clock kill) before saving results, via async-signal-safe faulthandler.dump_traceback(). - enable() additionally captures native fatal signals (SIGSEGV/SIGABRT/...). Gated by --periodic-hang-traceback (default off), enabled in the L0 stage command next to --periodic-save-unfinished-test. Complements surfacing the culprit test: the sidecar file gives the "where" to pair with the "who". Note: the plugin runs controller-side, so the watchdog covers non-xdist stages (where the controller is the test process); xdist-worker coverage is future work. Signed-off-by: Bowen Fu <5812640+BowenFu@users.noreply.github.com>
|
/bot run |
16a1079 to
8e0a537
Compare
|
PR_Github #59394 [ run ] triggered by Bot. Commit: |
|
PR_Github #59394 [ run ] completed with state
|
|
/bot run |
|
PR_Github #59416 [ run ] triggered by Bot. Commit: |
|
PR_Github #59416 [ run ] completed with state
|
ZhanruiSunCh
left a comment
ZhanruiSunCh
left a comment
There was a problem hiding this comment.
LGTM for infra part.
|
/bot run |
|
PR_Github #59980 [ run ] triggered by Bot. Commit: |
|
PR_Github #59980 [ run ] completed with state
|
|
/bot run |
|
PR_Github #60305 [ run ] triggered by Bot. Commit: |
|
PR_Github #60305 [ run ] completed with state
|
|
/bot run |
|
PR_Github #60550 [ run ] triggered by Bot. Commit: |
|
PR_Github #60550 [ run ] completed with state
|
|
/bot run |
|
PR_Github #60597 [ run ] triggered by Bot. Commit: |
|
PR_Github #60597 [ run ] completed with state
|
|
/bot run |
|
PR_Github #60910 [ run ] triggered by Bot. Commit: |
|
PR_Github #60910 [ run ] completed with state
|
7 participants
Summary by CodeRabbit
New Features
CI Improvements
Description
A per-test timeout or stage wall-clock kill leaves only an empty
Test terminated unexpectedlyrecord — we know which test hung (viaunfinished_test.txt, #16358) but not where. Seen on L0 #47641DGX_B200-PyTorch-6:test_kv_cache_v2_scheduler.py::TestKVCacheV2LoRA::test_lora_v2hung the full--timeout=3600window and was hard-killed; no traceback survived.This adds opt-in hang-traceback dumping to the periodic-junit plugin:
threading.Timerarmed per test dumps every thread's stack tooutput-dir/hang_traceback.txtat ~90% of the timeout, just before the kill.faulthandler.register(SIGINT/SIGTERM, chain=True)dumps at C level on a wall-clock kill even when the main thread is stuck in native CUDA, then chains to the existing report-saver.Gated by
--periodic-hang-traceback(default off), enabled in the L0 stage command. Controller-side only, so it covers non-xdist stages; xdist-worker coverage is future work.Sample output
Before:
test_lora_v2 FAILED "Test terminated unexpectedly"(stdout/stderr empty).After —
hang_traceback.txt(illustrative):Test Coverage
tests/integration/defs/utils/test_periodic_junit.py— timeout resolution (marker/global), hang dump, cancel-on-complete, no-arm-without-timeout. Pure-Python, no GPU.PR Checklist