Repository navigation
[https://nvbugs/6913451][fix] Avoid heavy imports in MPI shutdown - #20434
Conversation
2634a5f to
53bc92a
Compare
|
/bot run |
|
PR_Github #78241 [ run ] triggered by Bot. Commit: |
53bc92a to
aa8e105
Compare
aa8e105 to
1906eaf
Compare
|
PR_Github #78241 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. ℹ️ Recent review info
WalkthroughThe change adds a standalone authenticated MPI shutdown helper and routes launcher shutdown through it when available. It also updates installation, tests, and documentation for helper behavior, fallback conditions, timeouts, and exit-status handling. ChangesMPI shutdown flow
Priority: Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Launcher
participant StopHelper
participant ZeroMqQueue_PAIR_server
Launcher->>StopHelper: Start guarded shutdown helper
StopHelper->>ZeroMqQueue_PAIR_server: Send authenticated shutdown request
ZeroMqQueue_PAIR_server->>StopHelper: Return acknowledgment
StopHelper->>Launcher: Return helper status
Possibly related PRs
Merge Risk:
|
|
PR_Github #78337 [ run ] triggered by Bot. Commit: |
|
PR_Github #78337 [ run ] completed with state
|
1906eaf to
6c62b79
Compare
|
PR_Github #79402 [ run ] triggered by Bot. Commit: |
|
PR_Github #79359 [ run ] completed with state |
Thanks for the heads up @chenfeiz0326 . Updated to depend on #20608 and preserve its acknowledgment/retry protocol. Added reconnect and protocol compatibility coverage. |
|
PR_Github #79402 [ run ] completed with state
|
05a5433 to
333d35f
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #79942 [ run ] triggered by Bot. Commit: |
|
PR_Github #79942 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #80250 [ run ] triggered by Bot. Commit: |
|
PR_Github #80250 [ run ] completed with state Resource maintenance in LLM/main/L0_Test-x86_64-Multi-GPU #2924: B200 multi-GPU stages are paused to relieve the CI queue backlog (109 running / 459 pending on 2026-10-22). Owner: @sharabeshj-nvda. The CI team decides when to lift it.
|
237640b to
697dd78
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #80654 [ run ] triggered by Bot. Commit: |
|
PR_Github #80654 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
1 similar comment
|
/bot run --disable-fail-fast |
|
PR_Github #80763 [ run ] triggered by Bot. Commit: |
|
PR_Github #80763 [ run ] completed with state |
697dd78 to
dddb527
Compare
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
dddb527 to
b501a43
Compare
|
/bot skip --comment "Full CI pipeline passed earlier before the rebase." |
|
PR_Github #80935 [ skip ] triggered by Bot. Commit: |
|
PR_Github #80935 [ skip ] completed with state |
Summary
Avoid importing TensorRT-LLM again just to stop the MPI launcher, reducing shutdown overhead implicated in NVBUGs 6913451, 6964779 and 6964799. Preserve main's acknowledged shutdown protocol and lifecycle supervision.
Scope
pyzmq, scrubs the session key, authenticates replies without unpickling them, and retries on fresh PAIR connections.submit/NVBUG6964779 andtask_kwargs/NVBUG6964799 waivers. This restores eligibility for five remote-MPI variants in ARM/x86 CPU CI and, after merge, on main. Keep the separatesubmit_syncwaiver for NVBUG6988813.Verification
Latest CI: 66504 SUCCESS at pre-rebase head
697dd78196. It reused both CPU stages from 66405, where raw JUnit/logs confirm 154 affected cases passed and 1 skipped per architecture (ARM/x86), with no selected failures or retries. Coverage includes helper, launcher, supervisor, packaging, production queue/credential compatibility and five remote-MPI variants.submit_syncwas skipped under NVBUG6988813. Run 66405's DeepSeek-V4 CUDA OOM and H100 infrastructure failures remain part of its history; the subsequent successful rerun is not fresh CPU execution.Bug mapping below refers to
test_mpi_session.py::test_remote_mpi_session[variant]in 66405 at697dd78196, using the then-current 60-second shell watchdog:submit,flashinfer_workspace,flashinfer_temporary_cleanup,task_kwargs,client_sys_pathpassed; logs show successful task, helper, workers and server exits.submit_syncevidence or paired baseline timing.submitpassed on both architectures without its specific waiver.task_kwargspassed on both architectures without its specific waiver.Current head
b501a43274on mainab95e84be: applicable hooks, DCO and self-review passed. A local check using the production parser, waiver matcher and CPU stage-selection code confirms five eligible remote-MPI variants per architecture, with onlysubmit_syncskipped for NVBUG6988813. Local macOS ARM64/Python 3.13 helper, launcher, supervisor and packaging tests passed 108 cases with 1 Linux-only skip at77f1e77cd0; the current revision changes only waiver selection relative to that run; all tested code and test/configuration files are unchanged. Native MPI/full CI were not rerun after this rebase. Main's merged #20467 changes MPI task distribution and replaces the shell watchdog with a 120-second Python subprocess bound; those upstream runtime/test changes are preserved; this PR removes the blanket waiver to resume coverage. Earlier native results do not verify this new combination or prove all three intermittent bugs resolved. Repeated comparable ARM task-exit-to-launcher-exit timing remains outstanding.Notes
Main already includes #20608 for acknowledged stop delivery and #20232 for lifecycle supervision; no separate parent stack remains. Adapted from #20293.
After merge, update all three NVBUGs with the merged revision, corresponding evidence and remaining gaps; assess closure separately for each bug.