[https://nvbugs/6480621][test] Revert to 60-second KV transfer timeout for GB300 DeepSeek V4 Pro disaggregated perf-sanity - #17137
Conversation
|
/bot run --disable-fail-fast --stage-list "GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-2" |
|
/bot run --disable-fail-fast --disable-reuse-test --stage-list "GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-2" |
|
PR_Github #63105 [ run ] triggered by Bot. Commit: |
|
PR_Github #63106 [ run ] triggered by Bot. Commit: |
|
PR_Github #63105 [ run ] completed with state |
chienchunhung
changed the title
|
PR_Github #63106 [ run ] completed with state |
|
/bot run --disable-fail-fast --disable-reuse-test --stage-list "GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-1" |
|
PR_Github #63144 [ run ] triggered by Bot. Commit: |
|
PR_Github #63144 [ run ] completed with state
|
|
/bot run --disable-fail-fast --disable-reuse-test --stage-list "GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-1" |
|
PR_Github #63150 [ run ] triggered by Bot. Commit: |
|
PR_Github #63150 [ run ] completed with state
|
|
/bot run --disable-fail-fast --disable-reuse-test --stage-list "GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-1" |
|
PR_Github #63165 [ run ] triggered by Bot. Commit: |
|
PR_Github #63165 [ run ] completed with state
|
38ca970 to
3e6c120
Compare
|
/bot run --disable-fail-fast --disable-reuse-test --stage-list "GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-1" |
|
PR_Github #63555 [ run ] triggered by Bot. Commit: |
|
PR_Github #63555 [ run ] completed with state
|
3e6c120 to
b7bd98a
Compare
|
/bot run --disable-fail-fast --disable-reuse-test --stage-list "GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-2" |
|
PR_Github #63564 [ run ] triggered by Bot. Commit: |
|
PR_Github #63564 [ run ] completed with state |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
f2d7cb6 to
1714136
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/bot run --disable-fail-fast |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/unittest/others/test_cache_transceiver_precheck_run.py (2)
83-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a large exact-boundary case to the parametrization.
Only the
1024case allocates the extra reserve page.1024is a multiple oftokens_per_block, sonum_allocatedis 9 whilenum_prompt_blocksis 8.
7408is not a multiple of 128.num_allocatedandnum_prompt_blocksare both 58, so that case would still pass if the trim in_request_block_viewswere removed. Add a large exact-boundary length, such as7424(58 blocks, 59 allocated), to protect the boundary the stacked fix targets.♻️ Proposed parametrization change
-@pytest.mark.parametrize(("prompt_len", "expected_blocks"), ((1024, 8), (7408, 58))) +@pytest.mark.parametrize(("prompt_len", "expected_blocks"), ((1024, 8), (7408, 58), (7424, 58)))🤖 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 `@tests/unittest/others/test_cache_transceiver_precheck_run.py` around lines 83 - 111, Extend the parametrization of test_request_block_views_excludes_untransferred_speculative_page with a large exact-boundary prompt length such as 7424 and expect 58 verified blocks. Keep the existing 1024 and 7408 cases unchanged so the test covers both reserve-page allocation and the large exact-boundary trim in _request_block_views.
454-464: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
DISAGG_TRANS_ERRORstate branch.Request 1 is in the
failedlist here, soctx_finish_waveraises at thefailed_pairscheck. The later branch that inspectsreq.state == self.llm_request_state.DISAGG_TRANS_ERRORis never reached by any test in this file.Add a case where block-all reports the rid as completed but the request state is the error state. That path also must retain pages.
💚 Proposed added test
def test_ctx_finish_wave_retains_pages_on_disagg_trans_error_state(monkeypatch): runner, events = _ctx_finish_runner(monkeypatch, lambda _n: ([101], [])) reqs = {0: types.SimpleNamespace(py_request_id=101, state="error")} with pytest.raises(rp._TransferError, match=r"ctx DISAGG_TRANS_ERROR on pairs \[0\]"): runner.ctx_finish_wave(reqs) assert events == []🤖 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 `@tests/unittest/others/test_cache_transceiver_precheck_run.py` around lines 454 - 464, Add a test alongside test_ctx_finish_wave_retains_pages_when_request_failed that makes block-all report the request as completed while its state is DISAGG_TRANS_ERROR, using _ctx_finish_runner with no failed IDs and a request for pair 0. Assert ctx_finish_wave raises the DISAGG_TRANS_ERROR-specific _TransferError and that events remains empty, confirming pages are retained.
🤖 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.
Nitpick comments:
In `@tests/unittest/others/test_cache_transceiver_precheck_run.py`:
- Around line 83-111: Extend the parametrization of
test_request_block_views_excludes_untransferred_speculative_page with a large
exact-boundary prompt length such as 7424 and expect 58 verified blocks. Keep
the existing 1024 and 7408 cases unchanged so the test covers both reserve-page
allocation and the large exact-boundary trim in _request_block_views.
- Around line 454-464: Add a test alongside
test_ctx_finish_wave_retains_pages_when_request_failed that makes block-all
report the request as completed while its state is DISAGG_TRANS_ERROR, using
_ctx_finish_runner with no failed IDs and a request for pair 0. Assert
ctx_finish_wave raises the DISAGG_TRANS_ERROR-specific _TransferError and that
events remains empty, confirming pages are retained.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9fc46729-afee-487d-9cda-fe1261acdbf7
📒 Files selected for processing (13)
jenkins/scripts/perf/local/submit.pyjenkins/scripts/perf/submit.pytensorrt_llm/_torch/disaggregation/native/transfer.pytensorrt_llm/_torch/disaggregation/transceiver.pytests/scripts/perf-sanity/cache_transceiver_precheck/README.mdtests/scripts/perf-sanity/cache_transceiver_precheck/precheck_config.pytests/scripts/perf-sanity/cache_transceiver_precheck/run_precheck.pytests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con180_ctx3_dep4_gen1_dep32_eplb384_mtp3_ccb-NIXL.yamltests/unittest/disaggregated/test_cache_transceiver_precheck_e2e.pytests/unittest/disaggregated/test_transceiver_bounded_polling.pytests/unittest/others/test_cache_transceiver_precheck_config.pytests/unittest/others/test_cache_transceiver_precheck_run.pytests/unittest/scripts/test_perf_submit.py
🚧 Files skipped from review as they are similar to previous changes (11)
- jenkins/scripts/perf/local/submit.py
- tests/scripts/perf-sanity/cache_transceiver_precheck/precheck_config.py
- tensorrt_llm/_torch/disaggregation/native/transfer.py
- jenkins/scripts/perf/submit.py
- tests/unittest/disaggregated/test_cache_transceiver_precheck_e2e.py
- tests/scripts/perf-sanity/cache_transceiver_precheck/README.md
- tests/unittest/scripts/test_perf_submit.py
- tensorrt_llm/_torch/disaggregation/transceiver.py
- tests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con180_ctx3_dep4_gen1_dep32_eplb384_mtp3_ccb-NIXL.yaml
- tests/scripts/perf-sanity/cache_transceiver_precheck/run_precheck.py
- tests/unittest/disaggregated/test_transceiver_bounded_polling.py
|
PR_Github #63870 [ run ] triggered by Bot. Commit: |
|
PR_Github #63870 [ run ] completed with state
|
|
The That matters because the two are in very different review states. #17223 is
Separately,
Worth sorting out between you before either merges. Also: |
brnguyen2
left a comment
brnguyen2
left a comment
There was a problem hiding this comment.
Approving — the comments below are optional touch-ups, not blockers.
Reviewed the prerequisite commit as the real content; the YAML edit itself is fine.
One framing note for the PR description: kv_transfer_timeout_ms and the sender wait slice are different knobs. The YAML value feeds rx_timeout_s and the py_executor request-cancel deadline (py_executor.py:6159); the sender's slice is kv_transfer_sender_future_timeout_ms (default 1000ms). So the revert restores the 60s request cancellation deadline, and it's safe now because wait_complete observes cancellation between slices rather than mistaking a slice expiry for completion. Worth saying that way in the commit message — "600s → 60s" alone reads like the thing that was failing is being re-enabled unchanged.
The main leftover is that TxSession.wait_complete can no longer return TIMEOUT, which leaves dead plumbing in check_context_transfer_status and removes the last diagnostic for a wedged sender (details inline). Otherwise the ownership contract is clearly stated and the unit tests cover the cases that matter — cancel-between-slices, failed sibling behind a pending task, and retain-on-failure for both ctx and gen.
| for task in self.kv_tasks: | ||
| if not task.wait(timeout=self._timeout_s): | ||
| return WaitResult.TIMEOUT | ||
| while not task.wait(timeout=wait_slice_s): |
There was a problem hiding this comment.
(to be fixed in PR #17223, not here) This loop has no diagnostic at all. Previously a stuck task produced a TIMEOUT and a warning; now it spins on 1s slices silently. The only escape is the executor's request-cancel deadline (kv_transfer_timeout_ms, default 60000) — if a deployment sets that to null, check_context_transfer_status(None) blocks the executor thread forever with nothing in the log.
Suggest logging inside the loop on an escalating cadence, e.g. warn once past N slices with the elapsed time and rid, so a wedged NIXL write is diagnosable from a CI log without a stack dump.
| @@ -703,8 +703,9 @@ def check_context_transfer_status( | |||
| elif result is None: | |||
| continue | |||
| elif result == WaitResult.TIMEOUT: | |||
There was a problem hiding this comment.
(to be fixed in PR #17223, not here) This branch is now unreachable for TxSession: the blocking path loops until terminal and the non-blocking path returns None, so wait_complete never yields TIMEOUT. That makes timed_out, its _ctx_consensus_outcome argument, and this (now debug-level) message dead code.
Either delete the branch and the timed_out plumbing, or keep it and make wait_complete actually return TIMEOUT after some bounded number of slices. Leaving a permanently-false branch that also happens to be the only place a stalled sender was ever logged is the worst of both.
| if not self.aux_task.wait(timeout=self._timeout_s): | ||
| return WaitResult.TIMEOUT | ||
| if self._need_aux: | ||
| if self.aux_task is None: |
There was a problem hiding this comment.
(to be fixed in PR #17223, not here) A blocking=True call returning None is surprising for the caller: check_context_transfer_status hits elif result is None: continue, so the rid stays in _send_sessions and block-all returns claiming it drained everything. Is aux_task guaranteed to be set before the first wait_complete when _need_aux is true? If it's a transient window this is fine but should say so; if it can persist, this is a silent stuck-request path (the precheck catches it as missing, production won't).
| # block boundary those tokens allocate an additional page, but the | ||
| # transceiver intentionally trims its slice to prompt_len blocks. | ||
| # Verify the same payload range instead of the untransferred page. | ||
| valid = [b for b in blocks if b >= 0][:num_prompt_blocks] |
There was a problem hiding this comment.
(to be fixed in PR #17223, not here) The [:num_prompt_blocks] slice silently absorbs the under-allocated case too: if the KV manager hands back fewer valid blocks than the prompt needs — a real bug this precheck exists to catch — verification just checks fewer blocks and passes.
Add an explicit check, e.g. if len(valid) < num_prompt_blocks: raise/return a mismatch detail naming the layer and the counts. Only the extra trailing speculative page should be trimmed.
| key, value = token.split("=", 1) | ||
| if key == name: | ||
| return value | ||
| raise ValueError(f"pytestCommand does not set leading environment variable {name}") |
There was a problem hiding this comment.
(to be fixed in PR #17223, not here) This raises when LLM_MODELS_ROOT isn't a leading assignment in pytestCommand. Today getPytestBaseCommandLine (L0_Test.groovy:1358) always emits it third, so it works — but this turns a precheck-config detail into a hard abort of the whole disagg perf submission if that Groovy list is ever reordered or the var moves into envVarsToExport. Consider falling back to os.environ.get("LLM_MODELS_ROOT") before raising, or at least referencing the Groovy site in the error message so the next person knows where to look.
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>
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>
1714136 to
6e9981a
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/bot run --disable-fail-fast --disable-reuse-test --stage-list "GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-2" |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
tests/scripts/perf-sanity/cache_transceiver_precheck/precheck_config.py (1)
196-201: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReject an empty
llm_models_rootas well.The guard only rejects
None. An empty string passes and producesexport LLM_MODELS_ROOT=''. The precheck then fails later on the cluster with anINIT_ERRORfromresolve_model_dirreturningNone. A falsy check fails at launch-script generation time with the precise cause.♻️ Proposed fix
if enabled: - if llm_models_root is None: + if not llm_models_root: raise ValueError("enabled cache-transceiver precheck requires LLM_MODELS_ROOT")🤖 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 `@tests/scripts/perf-sanity/cache_transceiver_precheck/precheck_config.py` around lines 196 - 201, Update the guard in the enabled cache-transceiver precheck configuration path to reject both None and empty llm_models_root values before generating the export line. Preserve the existing ValueError message and top-level LLM_MODELS_ROOT assignment for valid non-empty values.tests/unittest/others/test_cache_transceiver_precheck_run.py (1)
83-112: 📐 Maintainability & Code Quality | 🔵 TrivialTest coverage summary (tests/ path instruction).**
Changed test functions
tests/unittest/others/test_cache_transceiver_precheck_run.py— added:test_request_block_views_excludes_untransferred_speculative_page,test_resolve_model_prefs_allows_registered_class_without_defaults_hook, sixctx_finish_wavecases, twoctx_run_wavecases, sevengen_run_wavecases, two_hard_abort_unquiescedcases, two peer-loop cases.
tests/unittest/disaggregated/test_cache_transceiver_harness.py— added:TestRequestTransferOwnershipFlow(7 cases) andTestTransferOwnershipHelpers(7 cases), plus the AST loaders and fakes.
tests/unittest/disaggregated/test_transceiver_bounded_polling.py— added: bounded-slice, shared-deadline, terminal-wins-at-deadline, timeout-fallback, timeout-retention, TP/PP consensus, and timeout-wiring cases; removed the former default-blocking wait test.
tests/unittest/others/test_cache_transceiver_precheck_config.py— added model-root quoting and enable-policy cases.Test list membership
All changed test files are unit tests under
tests/unittest/. They are not integration tests, so entries undertests/integration/test_lists/test-db/orqa/do not apply. The one integration-adjacent file,tests/unittest/disaggregated/test_cache_transceiver_precheck_e2e.py, requires a GPU. Confirm it is reachable from an existing CI stage rather than only runnable by hand.Verdict: needs follow-up
Coverage of the ownership contract is strong. The release ordering (
block_all→cuda_sync→peer_ack→free), page retention on every failure class, timeout propagation, and finalizer skipping are all asserted directly. Two gaps remain:
- No case covers the KV manager returning fewer valid blocks than the prompt needs. Both parameterizations here supply enough or extra blocks. That is the under-allocation hole flagged in
run_precheck.py. Add a case once that check lands.- No case asserts that a repeated block-all after a
TIMEOUTstays nonterminal at the transceiver level.test_tx_session_blocking_wait_times_out_stalled_task_and_does_not_reset_deadlinecovers the session, not the caller loop.As per path instructions: "Always produce a test coverage summary, even if no issues are found" and "A coverage verdict: sufficient, insufficient, or needs follow-up."
Do you want me to write the two missing test cases?
🤖 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 `@tests/unittest/others/test_cache_transceiver_precheck_run.py` around lines 83 - 112, The test coverage is missing under-allocation handling and transceiver-level repeated block-all behavior after TIMEOUT. Add one test for _request_block_views (or the relevant precheck validation) where the KV manager returns fewer valid blocks than the prompt requires, and add one caller-loop test asserting a repeated block-all after TIMEOUT remains nonterminal; preserve existing session-level coverage and follow current test helpers and parametrization patterns.Source: Path instructions
tests/unittest/disaggregated/test_cache_transceiver_harness.py (1)
61-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared AST loader.
_load_driver_ownership_helpersand_load_driver_request_floware near-identical. They differ only in the module name,selected_names, and the stand-in namespace. Roughly 50 lines are duplicated, including the parse, the node filter, theexec, and the missing-symbol assertion. A rename in the driver now requires editing two selection sets and two stand-in dicts.The
missingassertion is the right safeguard for this loading technique. Keep it in the extracted helper.♻️ Proposed refactor
+def _load_driver_symbols(module_name: str, selected_names: set, namespace: dict) -> types.ModuleType: + """Load selected top-level driver symbols without importing GPU/MPI runtime modules.""" + source = Path(DRIVER_SCRIPT).read_text() + tree = ast.parse(source, filename=DRIVER_SCRIPT) + selected = [ + node + for node in tree.body + if isinstance(node, (ast.ClassDef, ast.FunctionDef)) and node.name in selected_names + ] + module = types.ModuleType(module_name) + module.__dict__.update(namespace) + exec( + compile(ast.Module(body=selected, type_ignores=[]), DRIVER_SCRIPT, "exec"), + module.__dict__, + ) + missing = selected_names - set(module.__dict__) + assert not missing, f"{module_name}: driver symbols not loaded: {sorted(missing)}" + return moduleThen both loaders reduce to a
selected_namesset, a namespace dict, and one call.🤖 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 `@tests/unittest/disaggregated/test_cache_transceiver_harness.py` around lines 61 - 172, Extract the common AST parsing, top-level node selection, module creation, execution, and missing-symbol assertion from _load_driver_ownership_helpers and _load_driver_request_flow into a shared loader helper. Keep each existing loader responsible only for its selected_names and stand-in namespace, pass those values to the helper, and preserve the missing-symbol safeguard and current module behavior.examples/disaggregated/slurm/cache_transceiver_test/run_cache_transceiver_test.py (1)
411-461: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoffConsider authenticated framing for the new release handshake.
_exchange_release_decisionadds a second pickle exchange on the leader-to-leader ZMQ socket.pickle.loadsexecutes arbitrary code from the payload. The socket bindstcp://*:<zmq_port>with no authentication, so any process that can reach the context node's port can drive this decode.The sibling driver
tests/scripts/perf-sanity/cache_transceiver_precheck/run_precheck.pyalready solved this. It frames control messages with an HMAC key exchanged through a 0600 work-dir file (pack_msg/unpack_msg). The payload here is a(status, reason)string pair, so JSON plus the same HMAC framing would carry it without pickle.This harness is a tuning example, not production serving, so the risk is bounded by the cluster network. Aligning the two drivers still removes the divergence and the code-execution surface.
🤖 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 `@examples/disaggregated/slurm/cache_transceiver_test/run_cache_transceiver_test.py` around lines 411 - 461, Replace the unauthenticated pickle send/receive calls in _exchange_release_decision with the authenticated JSON/HMAC framing used by pack_msg and unpack_msg in the sibling precheck driver. Reuse the 0600 work-directory key exchange and validate the decoded payload as a (status, reason) string pair before applying the existing handshake logic; preserve timeout and fatal-error handling.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In
`@examples/disaggregated/slurm/cache_transceiver_test/run_cache_transceiver_test.py`:
- Around line 411-461: Replace the unauthenticated pickle send/receive calls in
_exchange_release_decision with the authenticated JSON/HMAC framing used by
pack_msg and unpack_msg in the sibling precheck driver. Reuse the 0600
work-directory key exchange and validate the decoded payload as a (status,
reason) string pair before applying the existing handshake logic; preserve
timeout and fatal-error handling.
In `@tests/scripts/perf-sanity/cache_transceiver_precheck/precheck_config.py`:
- Around line 196-201: Update the guard in the enabled cache-transceiver
precheck configuration path to reject both None and empty llm_models_root values
before generating the export line. Preserve the existing ValueError message and
top-level LLM_MODELS_ROOT assignment for valid non-empty values.
In `@tests/unittest/disaggregated/test_cache_transceiver_harness.py`:
- Around line 61-172: Extract the common AST parsing, top-level node selection,
module creation, execution, and missing-symbol assertion from
_load_driver_ownership_helpers and _load_driver_request_flow into a shared
loader helper. Keep each existing loader responsible only for its selected_names
and stand-in namespace, pass those values to the helper, and preserve the
missing-symbol safeguard and current module behavior.
In `@tests/unittest/others/test_cache_transceiver_precheck_run.py`:
- Around line 83-112: The test coverage is missing under-allocation handling and
transceiver-level repeated block-all behavior after TIMEOUT. Add one test for
_request_block_views (or the relevant precheck validation) where the KV manager
returns fewer valid blocks than the prompt requires, and add one caller-loop
test asserting a repeated block-all after TIMEOUT remains nonterminal; preserve
existing session-level coverage and follow current test helpers and
parametrization patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7c7cb1e4-dfdc-4d43-ba6a-fe04def30db0
📒 Files selected for processing (17)
examples/disaggregated/slurm/cache_transceiver_test/README.mdexamples/disaggregated/slurm/cache_transceiver_test/config.yamlexamples/disaggregated/slurm/cache_transceiver_test/run_cache_transceiver_test.pyjenkins/scripts/perf/local/submit.pyjenkins/scripts/perf/submit.pytensorrt_llm/_torch/disaggregation/native/transfer.pytensorrt_llm/_torch/disaggregation/transceiver.pytests/scripts/perf-sanity/cache_transceiver_precheck/README.mdtests/scripts/perf-sanity/cache_transceiver_precheck/precheck_config.pytests/scripts/perf-sanity/cache_transceiver_precheck/run_precheck.pytests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con180_ctx3_dep4_gen1_dep32_eplb384_mtp3_ccb-NIXL.yamltests/unittest/disaggregated/test_cache_transceiver_harness.pytests/unittest/disaggregated/test_cache_transceiver_precheck_e2e.pytests/unittest/disaggregated/test_transceiver_bounded_polling.pytests/unittest/others/test_cache_transceiver_precheck_config.pytests/unittest/others/test_cache_transceiver_precheck_run.pytests/unittest/scripts/test_perf_submit.py
🚧 Files skipped from review as they are similar to previous changes (5)
- jenkins/scripts/perf/local/submit.py
- tests/unittest/disaggregated/test_cache_transceiver_precheck_e2e.py
- tests/unittest/scripts/test_perf_submit.py
- jenkins/scripts/perf/submit.py
- tensorrt_llm/_torch/disaggregation/native/transfer.py
|
PR_Github #64469 [ run ] triggered by Bot. Commit: |
|
PR_Github #64469 [ run ] completed with state
|
6 participants
Summary
Stacked on #17223. This PR changes only the targeted GB300 DeepSeek V4 Pro perf-sanity YAML:
kv_transfer_timeout_msfrom 600000 ms to 60000 ms;cache_transceiver_precheckfor this target while the gate is globally opt-in.The explicit opt-in ensures the targeted stage exercises #17223's ownership/deadline path before running the real 60-second workload.
Stack and merge order
f807789a56be79c4928b8a4e674ffa26681e62e26e9981a12025b8c612438a15a7366c0c4f1f0815main; the remaining diff should be this single YAML file.Validation
Exact stage:
GB300-44_GPUs-11_Nodes-PyTorch-Disagg-PerfSanity-CTX3-NODE1-GPU4-GEN1-NODE8-GPU32-Post-Merge-2Exact test:
perf/test_perf_sanity.py::test_e2e[disagg_upload-gen_only-gb300_deepseek-v4-pro-fp4_8k1k_con180_ctx3_dep4_gen1_dep32_eplb384_mtp3_ccb-NIXL]A prior revision passed this stage with test reuse disabled (L0 #51530), including all six precheck peer/request-length combinations and 180/180 benchmark requests. That run is useful historical evidence but does not validate the current restacked SHA; a fresh targeted run is being triggered.
This CI proxy is 3 CTX servers and concurrency 180. It does not replace rerunning the original 8-CTX, concurrency-1760 NVBug workload.
Dev Engineer Review
kv_transfer_timeout_msfrom600000to60000for GEN and CTX in the targeted configuration.LLM_MODELS_ROOTexport.QA Engineer Review
Test-code changes include:
extract_pytest_command_envtests for quoted values, spaces,=characters, missing assignments, and malformed exports.test_tx_session_wait_complete_defaults_to_blocking.test-db/orqa/entries were added or modified.