[https://nvbugs/6388787][fix] Revert Pass IPC HMAC key through file descriptor (#15654) - #15961
Conversation
|
/bot run --disable-fail-fast --stage-list "*PerfSanity*" |
📝 WalkthroughWalkthroughThis PR replaces the file-descriptor-based mechanism for passing the spawn-proxy IPC HMAC key with a direct environment variable ( ChangesIPC HMAC key handling migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LaunchScript as trtllm-llmapi-launch
participant Env as Environment
participant ServeCmd as serve.py (leader)
participant ChildProc as Child process
participant Worker as GenerationExecutorWorker
LaunchScript->>Env: export TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY
ServeCmd->>Env: set TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY (secrets.token_hex)
ServeCmd->>ChildProc: start subprocess with non_mpi_env
ChildProc->>Env: read TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY
ChildProc->>Worker: initialize with hmac_key
Worker->>Worker: assert hmac_key is set
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@tensorrt_llm/executor/ipc.py`:
- Around line 43-49: The IPC docstring is stale because it still describes
disabling HMAC even though the constructor now always requires it. Update the
docstring in the Queue/IPC initializer to remove the unsupported
`use_hmac_encryption is False` case and clearly state that HMAC encryption is
always required, keeping the parameter description consistent with the assertion
in the initialization logic.
- Line 51: Replace the mandatory HMAC guard in the IPC path with a runtime
validation instead of an assert, since the check in the executor/ipc.py logic
can be removed under optimized Python runs. Update the relevant check around
use_hmac_encryption to explicitly raise ValueError when it is false, preserving
the security enforcement in all deployments.
In `@tensorrt_llm/executor/utils.py`:
- Around line 39-43: The HMAC key validation in the utility that reads
TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY must reject both missing and empty values
instead of relying on assert. Update the key-loading logic in the function that
calls os.getenv and bytes.fromhex so it uses the
LlmLauncherEnvs.TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY constant directly for the
environment lookup and performs an explicit empty-string check before converting
the key.
In `@tensorrt_llm/llmapi/trtllm-llmapi-launch`:
- Line 43: The launch script currently generates
TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY with openssl, which can silently yield an
empty value if openssl is unavailable. Update the trtllm-llmapi-launch logic to
avoid relying on openssl by generating the key with a safer source such as
secrets.token_hex(32), or explicitly check the result and fail fast before
export. Keep the change focused around the TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY
setup so get_spawn_proxy_process_ipc_hmac_key_env() never receives an empty key.
🪄 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: cd7fa32b-ace1-4786-ad4e-f819d25f2ea2
📒 Files selected for processing (7)
tensorrt_llm/commands/serve.pytensorrt_llm/executor/ipc.pytensorrt_llm/executor/utils.pytensorrt_llm/executor/worker.pytensorrt_llm/llmapi/trtllm-llmapi-launchtests/unittest/executor/test_ipc.pytests/unittest/executor/test_launcher_envs.py
💤 Files with no reviewable changes (2)
- tests/unittest/executor/test_launcher_envs.py
- tests/unittest/executor/test_ipc.py
|
PR_Github #57708 [ run ] triggered by Bot. Commit: |
|
PR_Github #57709 [ run ] triggered by Bot. Commit: |
|
PR_Github #57708 [ run ] completed with state |
|
/bot run --disable-fail-fast --stage-list "DGX_B200-16_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU8-Post-Merge-1,DGX_B200-16_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU8-Post-Merge-2,GB200-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-3,GB200-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-4" |
|
PR_Github #57712 [ run ] triggered by Bot. Commit: |
|
PR_Github #57709 [ run ] completed with state |
|
PR_Github #57712 [ run ] completed with state
|
7d18710 to
2e3ba4d
Compare
|
/bot run --disable-fail-fast --stage-list "*PerfSanity*" |
|
PR_Github #57732 [ run ] triggered by Bot. Commit: |
|
PR_Github #57732 [ run ] completed with state
|
…escriptor (NVIDIA#15654) Reverts NVIDIA#15654 (48fc753). PR NVIDIA#15654 passes the IPC HMAC key from the parent process to the LLM-API proxy child via a file descriptor. In the QA-pytest Slurm/MPI launch path used by trtllm-bench and trtllm-serve, the fd handshake between Rank0's forked subshell (spawning internal mpirun) and the mgmn_leader_node RemoteMpiCommSessionServer deadlocks silently: - trtllm-bench (nvbugs/6388787): after "start MpiSession with N workers" the bench process emits no further stdout for ~30 min, workers stay alive at 0% GPU / 4 MiB VRAM, no exception, no MPI abort. Perf harness SIGKILLs at _STALL_TIMEOUT=1800s. - trtllm-serve (nvbugs/6405747): serve mode's /health endpoint never binds, and pytest fails with "Server http://localhost:<port>/health did not become ready within 3600s" on both baseline and candidate wheels. This is the third time the "Pass IPC HMAC key via fd" idea has broken bench/serve (previously PR NVIDIA#14378 was reverted by PR NVIDIA#14782 for the same class of failure: BlockingIOError in _read_spawn_proxy_process_ ipc_hmac_key_fd). Reverting until the fd inheritance across fork+exec into the mpirun child is verified end-to-end for both bench and serve launch paths. Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
2e3ba4d to
0745fba
Compare
…pre-merge job 46441 Removed 70 perf sanity waives that passed in pre-merge job 46441 (OpenSearch-verified). Kept 12 waives whose test cases did not appear in the job 46441 OpenSearch results (pytest failed before uploading data). Breakdown: - 70 removed (passed in OpenSearch job 46441) - 12 kept (not present in OpenSearch job 46441) Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
0745fba to
c514f81
Compare
|
/bot skip --comment "Only unwaive perf tests, no need to run the whole CI pipeline" |
|
PR_Github #57775 [ skip ] triggered by Bot. Commit: |
|
PR_Github #57775 [ skip ] completed with state |
…escriptor (NVIDIA#15654) (NVIDIA#15961) Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
Summary
Reverts #15654 (48fc753).
PR #15654 passes the IPC HMAC key from the parent process to the LLM-API proxy child via a file descriptor. In the QA-pytest Slurm/MPI launch path used by trtllm-bench and trtllm-serve, the fd handshake between Rank0's forked subshell (spawning internal
mpirun) and themgmn_leader_nodeRemoteMpiCommSessionServerdeadlocks silently.Symptoms
trtllm-bench (nvbugs/6388787 — bisected there): after
[llmapi] start MpiSession with N workersthe bench process emits no further stdout for ~30 min, workers stay alive at 0% GPU / 4 MiB VRAM, no exception, no MPI abort. Perf harness SIGKILLs at_STALL_TIMEOUT=1800s.trtllm-serve (nvbugs/6405747): the
/healthendpoint never binds and pytest fails withServer http://localhost:<port>/health did not become ready within 3600son both baseline and candidate wheels. Reproduced on nemotron_3_ultra_550b_nvfp4 serve mode, B200 EP4, on 2026-07-05.Prior history
This is the third time the "pass IPC HMAC key via fd" idea has broken bench/serve:
BlockingIOError: [Errno 11]in_read_spawn_proxy_process_ipc_hmac_key_fdBoth prior attempts had the same root cause: the writer's fd does not reliably survive the fork+exec into the internal
mpirunchild spawned bytrtllm-llmapi-launch, and the reader (mgmn_leader_node) can't completeos.read(fd, ...)before its ZMQ PAIRaccept()times out. Attempt #2 (this revert) fails silently instead of raising becauseos.set_blocking(fd, True)was added — same deadlock, no exception.Test plan
deepseek_v3.2_fp4-bench-pytorch-float4 ep:8) — should proceed paststart MpiSession with N workersand produce a throughput number instead of stalling to the perf-harness SIGKILL.nemotron_3_ultra_550b_nvfp4-serve(nvbugs/6405747 case) —/healthshould bind before pytest's 3600s server-ready timeout.Next steps
Reverting until the fd-inheritance across fork+exec into the mpirun child is verified end-to-end for both the bench and serve launch paths (not just the unit-test-level
test_launcher_envs.pycoverage that #15654 added).Signed-off-by: Chenfei Zhang chenfeiz@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes