[https://nvbugs/6410963][fix] Make cache transceiver transport reporting deterministic - #15893
Conversation
|
/bot run --disable-fail-fast |
📝 WalkthroughWalkthroughThis change updates a disaggregated cache transceiver test harness to manage two mpirun subprocesses via file-backed logging and explicit process-group lifecycle handling, replacing communicate()-based timeouts with deadline-based waits and SIGTERM/SIGKILL escalation. It also updates the UCX_PROTO_INFO environment value and related failure-message text. ChangesCache Transceiver Harness Test
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant CtxProcess
participant GenProcess
Test->>CtxProcess: launch mpirun (new session, log file)
Test->>GenProcess: launch mpirun (new session, log file)
Test->>Test: wait_for_processes until deadline
alt timeout occurs
Test->>CtxProcess: terminate process group (SIGTERM then SIGKILL)
Test->>GenProcess: terminate process group (SIGTERM then SIGKILL)
end
Test->>Test: read tailed logs from files
Test->>Test: assert results / build failure message with log tails
Related Issues: None found in provided context. Related PRs: None found in provided context. Suggested labels: tests, disaggregated-serving Suggested reviewers: None found in provided context. 🐰 A rabbit taps the process tree, 🚥 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: 1
🤖 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/unittest/disaggregated/test_cache_transceiver_harness.py`:
- Around line 102-108: The cleanup path in test_cache_transceiver_harness’s
process shutdown logic leaves proc.wait() unbounded after the SIGKILL loop,
which can still hang if a child is stuck during teardown. Update the wait in the
processes loop to use a timeout, following the bounded pattern already used by
terminate() in disagg_test_utils, and keep the existing
killpg/ProcessLookupError handling intact so the helper cannot block
indefinitely.
🪄 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: 48d6c89b-cfe3-4139-828c-6f58602511dc
📒 Files selected for processing (1)
tests/unittest/disaggregated/test_cache_transceiver_harness.py
|
/bot run --disable-fail-fast |
|
PR_Github #57270 [ run ] triggered by Bot. Commit: |
|
PR_Github #57270 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #57574 [ run ] triggered by Bot. Commit: |
|
PR_Github #57574 [ run ] completed with state
|
79c053d to
19fc7c0
Compare
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
19fc7c0 to
723dd63
Compare
|
/bot run --disable-fail-fast --stage-list "DGX_H100-PyTorch-4" |
|
PR_Github #57795 [ run ] triggered by Bot. Commit: |
|
PR_Github #57795 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #57833 [ run ] triggered by Bot. Commit: |
|
PR_Github #57833 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #57864 [ run ] triggered by Bot. Commit: |
|
PR_Github #57864 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #58040 [ run ] triggered by Bot. Commit: |
|
PR_Github #58040 [ run ] completed with state |
…VIDIA#15893) Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
…VIDIA#15893) Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
@coderabbitai summary
Description
test_single_node_transfercurrently setsUCX_PROTO_INFO=usedand requiresselected_transportto be non-empty.usedemits diagnostics during UCX worker teardown, so the output depends on the active MPI/UCX runtime and object lifecycle. After the PyTorch 26.04 image transition, the KV transfer still completes successfully, butreport.pyobserves no transport and the test fails.This has caused the exact test to fail in all 46 observed
DGX_H100-PyTorch-4runs, including unrelated PRs and main.Changes
UCX_PROTO_INFO=yintest_single_node_transferso protocol-selection tables are emitted deterministically.UCX_TLSvalues, including weighted UCX 1.21 multi-lane rows.Root Cause
The test and mandatory transport assertion were introduced by #14933. The PyTorch 26.04 runtime transition from #12643 exposed the teardown-dependent diagnostic behavior; it did not cause the KV transfer itself to fail.
Test Coverage
tests/unittest/disaggregated/test_cache_transceiver_harness_report.pytests/unittest/disaggregated/test_cache_transceiver_harness.py/bot run --disable-fail-fast --stage-list "DGX_H100-PyTorch-4"Related PRs
GitHub Bot Help
To see a list of available CI bot commands, comment
/bot help.