[TRTLLM-13550][feat] WideEP FT: add MPI signal handler replacement (1d.0)#14160
Conversation
…al handler)
Adds the upstream PR number for 1d.0 (MPI signal handler replacement) to
all six places that already track in-flight Phase 1 MVP PRs:
* README.md: new row in the In-flight PRs table.
* 00-executive-summary.md: Mode A line + Phase 1 MVP status row.
* 03-failure-modes-and-gaps.md: Mode A narrative + post-1d.0 inversion
(also opportunistically backfills the existing 1a.2 PR# in the same
Mode B sentence for symmetry).
* 05-phase-1-immediate-survival.md §5.4: Signal handler replacement
sub-section header (notes the env-var MVP gate vs the LLMArgs field
that lands in PR 1d.1).
* 08-implementation-plan.md: Status (April 2026) bullet + Gantt
annotation marking the row done.
* 09-risks-and-open-questions.md: Audit 1a Day 2 row.
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
…ype scaffold Adds PR NVIDIA#14198 (DO NOT SUBMIT — preview only) to the in-flight PRs table in the README and updates mvp-prototype-plan.md to reflect that the prototype scaffolding is now shipped on branch WideEP-FT/mvp-prototype: * README.md In-flight PRs table: new row for NVIDIA#14198 (status flagged as "Draft (DO NOT SUBMIT — preview only)" to distinguish from the MVP component PRs). * README.md prototype paragraph: links NVIDIA#14198 alongside the cherry-picked NVIDIA#13302 and NVIDIA#14160; notes the discardable prototypes/wide_ep_ft_mvp/ directory. * mvp-prototype-plan.md Status header: bumped to reflect scaffolding-shipped state. * mvp-prototype-plan.md §6 sequencing: new "Current status (2026-05-15)" paragraph documenting that both 1a.1 (NVIDIA#13302) and 1d.0 (NVIDIA#14160) are private cherry-picks on the prototype branch pending their merges to main, and that the kernel-side 1a.2 + 1a.3 work is deferred per prototypes/wide_ep_ft_mvp/kernel/README.md. Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
d0e705d to
72c1d8e
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #53997 [ run ] triggered by Bot. Commit: |
|
PR_Github #53997 [ run ] completed with state
|
72c1d8e to
78dc051
Compare
|
/bot run --disable-fail-fast --stage-list "x86_64-Linux,SBSA-Linux" |
|
PR_Github #54022 [ run ] triggered by Bot. Commit: |
|
PR_Github #54022 [ run ] completed with state
|
78dc051 to
93cb089
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54149 [ run ] triggered by Bot. Commit: |
|
PR_Github #54149 [ run ] completed with state
|
93cb089 to
cca3484
Compare
|
/bot run |
|
PR_Github #54377 [ run ] triggered by Bot. Commit: |
|
PR_Github #54377 [ run ] completed with state
|
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
cca3484 to
2ba2d25
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54484 [ run ] triggered by Bot. Commit: |
|
PR_Github #54484 [ run ] completed with state
|
|
/bot run --disable-fail-fast --stage-list "DGX_H100-PyTorch-2,H100_PCIe-PyTorch-Ray-1" |
|
PR_Github #54634 [ run ] triggered by Bot. Commit: |
|
PR_Github #54634 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #54667 [ run ] triggered by Bot. Commit: |
|
PR_Github #54667 [ run ] completed with state |
📝 WalkthroughWalkthroughAdds two new public functions to the MPI utilities: ChangesWideEP MPI Fault-Tolerance Signal Handlers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cpp/include/tensorrt_llm/runtime/utils/mpiUtils.h (1)
1-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the copyright year to include 2026.
The copyright header shows
2021-2023, but this file is being modified in 2026. Per coding guidelines, the copyright header should include the year of the latest meaningful modification.Suggested fix
/* - * Copyright (c) 2021-2023, NVIDIA CORPORATION. All rights reserved. + * Copyright (c) 2021-2026, NVIDIA CORPORATION. All rights reserved. *🤖 Prompt for 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. In `@cpp/include/tensorrt_llm/runtime/utils/mpiUtils.h` around lines 1 - 15, Update the copyright year range in the header comment of mpiUtils.h from "2021-2023" to "2021-2026" to reflect the current modification year. Change the copyright line that currently reads "Copyright (c) 2021-2023" to "Copyright (c) 2021-2026" to comply with coding guidelines requiring the latest modification year.Source: Coding guidelines
cpp/tensorrt_llm/runtime/utils/mpiUtils.cpp (1)
1-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the copyright year to include 2026.
The copyright header shows
2022-2024, but this file is being modified in 2026.Suggested fix
/* - * Copyright (c) 2022-2024, NVIDIA CORPORATION. All rights reserved. + * Copyright (c) 2022-2026, NVIDIA CORPORATION. All rights reserved. *🤖 Prompt for 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. In `@cpp/tensorrt_llm/runtime/utils/mpiUtils.cpp` around lines 1 - 15, The copyright year range in the header comment shows "2022-2024" but needs to be updated to reflect the current year. Change the year range in the NVIDIA CORPORATION copyright line from "2022-2024" to "2022-2026" to accurately reflect that the file has been modified in 2026.Source: Coding guidelines
🧹 Nitpick comments (1)
cpp/tests/unit_tests/runtime/mpiUtilsTest.cpp (1)
26-27: 💤 Low valueConsider adding a platform guard or CMake condition for POSIX-only tests.
This test file includes POSIX-specific headers (
<sys/wait.h>,<unistd.h>) and usesfork()/waitpid()which have no Windows equivalents. The implementation file guards<unistd.h>with#ifndef _WIN32, but this test file would fail to compile on Windows.If Windows builds are in scope, consider either:
- Wrapping the entire file in
#ifndef _WIN32/#endif- Conditionally excluding the test target in CMakeLists.txt for Windows
🤖 Prompt for 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. In `@cpp/tests/unit_tests/runtime/mpiUtilsTest.cpp` around lines 26 - 27, The file cpp/tests/unit_tests/runtime/mpiUtilsTest.cpp includes POSIX-specific headers (sys/wait.h and unistd.h) and uses POSIX functions like fork() and waitpid() without any platform guards, which will cause compilation failures on Windows. To fix this, wrap the POSIX-specific includes and the entire test implementation with a preprocessor guard using `#ifndef` _WIN32 at the beginning of the file and `#endif` at the end to conditionally compile this test only on non-Windows platforms. Alternatively, if you prefer to keep the test file unchanged, you can add a CMake condition to exclude this test target from Windows builds in the CMakeLists.txt configuration. Choose one approach and apply it consistently.
🤖 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.
Outside diff comments:
In `@cpp/include/tensorrt_llm/runtime/utils/mpiUtils.h`:
- Around line 1-15: Update the copyright year range in the header comment of
mpiUtils.h from "2021-2023" to "2021-2026" to reflect the current modification
year. Change the copyright line that currently reads "Copyright (c) 2021-2023"
to "Copyright (c) 2021-2026" to comply with coding guidelines requiring the
latest modification year.
In `@cpp/tensorrt_llm/runtime/utils/mpiUtils.cpp`:
- Around line 1-15: The copyright year range in the header comment shows
"2022-2024" but needs to be updated to reflect the current year. Change the year
range in the NVIDIA CORPORATION copyright line from "2022-2024" to "2022-2026"
to accurately reflect that the file has been modified in 2026.
---
Nitpick comments:
In `@cpp/tests/unit_tests/runtime/mpiUtilsTest.cpp`:
- Around line 26-27: The file cpp/tests/unit_tests/runtime/mpiUtilsTest.cpp
includes POSIX-specific headers (sys/wait.h and unistd.h) and uses POSIX
functions like fork() and waitpid() without any platform guards, which will
cause compilation failures on Windows. To fix this, wrap the POSIX-specific
includes and the entire test implementation with a preprocessor guard using
`#ifndef` _WIN32 at the beginning of the file and `#endif` at the end to
conditionally compile this test only on non-Windows platforms. Alternatively, if
you prefer to keep the test file unchanged, you can add a CMake condition to
exclude this test target from Windows builds in the CMakeLists.txt
configuration. Choose one approach and apply it consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 488d33c9-9c64-4d45-bb3f-8be14ef395a6
📒 Files selected for processing (4)
cpp/include/tensorrt_llm/runtime/utils/mpiUtils.hcpp/tensorrt_llm/runtime/utils/mpiUtils.cppcpp/tests/unit_tests/runtime/CMakeLists.txtcpp/tests/unit_tests/runtime/mpiUtilsTest.cpp
chienchunhung
changed the title
…d.0) (NVIDIA#14160) Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com> Signed-off-by: GitLab CI Bot <gitlab-ci@nvidia.com>
…d.0) (NVIDIA#14160) Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
3 participants
Summary by CodeRabbit
New Features
Tests
Summary
Mode A fix from the WideEP fault-tolerance design (§5.4). Today, any rank hitting
SIGABRT/SIGSEGVcallsMPI_Abort(MPI_COMM_WORLD), which propagates throughmpirunand kills the entire 72-rank EP group within ~20s — before kernel rank-mask, EPLB remap, AlltoAll watchdog, or FT subcomm get a chance to run. This PR adds a non-propagating handler variant that calls_exit(137)instead.What this PR does
tensorrt_llm::mpi::isFaultToleranceModeEnabled()— readsTLLM_FAULT_TOLERANCE_MODE; enables iff value is the string"1".tensorrt_llm::mpi::installFaultToleranceSignalHandlers()— replaces the defaultSIGABRT/SIGSEGVhandlers with_exit(137). Async-signal-safe (_exitis in POSIX's safe set;MPI_Abortandprintfare not). Skipsatexit/Python finalizers/MPI_Finalize, all of which can deadlock on a poisoned state.mpi::initialize()— picks the new branch when the env var is set, overriding both the legacyMPI_Abortand theforwardAbortToParentkill(parent, SIGKILL)paths. The two existing branches are untouched.The
kill(parent, SIGKILL)skip in FT mode is deliberate: killing the parent defeats the entire purpose of letting survivors outlive a peer death.Scoping decisions
LLMArgsfieldLLMArgsfield is PR 1d.1 (depends on 1c.3/1c.4 which aren't landed yet). Documented as such in code comments.137128 + 9) so logs distinguish "rank died in FT mode" from generic crashes (EXIT_FAILURE = 1).SIGABRT+SIGSEGVSIGINT/SIGTERMdeliberately skipped (graceful-shutdown intent).--mca orte_enable_recovery 1not enforced hereTLLM_LOG_INFOreminder at initialize time; will be wired intotrtllm-serveas a follow-up.Tests
cpp/tests/unit_tests/runtime/mpiUtilsTest.cpp— 9 items:"1","0","", truthy synonyms)PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.