[None][infra] CBTS Layer 3: pass test-db via Artifactory instead of env var - #15142
Conversation
|
/bot run |
|
PR_Github #52969 [ run ] triggered by Bot. Commit: |
|
/bot kill |
|
/bot run |
📝 WalkthroughWalkthroughThis PR adds AttentionType enum infrastructure to disambiguate KV-cache layout semantics across backends (MHA vs MLA), wires it through all attention implementations, and introduces cache transceiver support for disaggregated AutoDeploy inference. Changes include persistent-cache validation, executor wiring, Jenkins CBTS tarball artifact handoff, configuration files, and comprehensive integration tests. ChangesDisaggregated inference and cache semantics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
…nv var
Previously cbts_input_json_b64 piggybacked the raw PR diff (base64-encoded,
up to 256 KB) onto the testFilter Jenkins parameter so each L0_Test agent
could re-run main.py locally to regenerate cbts_test_db/. When diffs were
large this pushed the process environment over Linux ARG_MAX (~2 MB), causing
git to fail with "error=7, Argument list too long" before agents could even
check out the pipeline script.
Fix: the orchestrator now tars the already-generated cbts_test_db/ and
uploads it to Artifactory (${UPLOAD_PATH}/cbts/cbts_test_db.tar.gz) after
main.py runs. Each agent downloads and extracts the tarball directly instead
of re-running main.py. The testFilter parameter no longer carries any large
payload, eliminating the ARG_MAX failure. Layer 3 per-test filtering is fully
preserved; agents fall back to the source test-db if the download fails.
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
|
PR_Github #52971 [ kill ] triggered by Bot. Commit: |
|
PR_Github #52969 [ run ] completed with state |
|
PR_Github #52971 [ kill ] completed with state |
|
@crazydemo the MR description is no longer accurate |
|
PR_Github #52972 [ run ] triggered by Bot. Commit: |
Thanks, updated. |
Add a top-level constant for the Artifactory base URL so it can be updated in one place when the server changes. Use it in the CBTS Layer 3 artifact download added in the previous commit. Existing call sites are left unchanged to keep this diff minimal. Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
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 `@jenkins/L0_MergeRequest.groovy`:
- Around line 778-787: The catch block around the CBTS artifact creation/upload
(the try that runs sh "tar ..." and trtllm_utils.uploadArtifacts) is swallowing
InterruptedException and allowing canceled builds to continue; update the catch
in the block that handles result.test_db_dir_override to detect
InterruptedException (or Thread.currentThread().isInterrupted()) and re-throw it
immediately before performing the fallback pipeline.echo so cancellations
propagate, leaving the existing fallback behavior for other Exception types
intact.
In `@tensorrt_llm/_torch/auto_deploy/shim/interface.py`:
- Around line 938-952: The code currently adds every entry from
ssm_managed/conv_managed (and their spec lists and replay_* tuples) into
managed_names even when only the first mamba_num_layers were actually bound by
the manager; change the updates to only whitelist the entries that were actually
bound by the manager (i.e., filter ssm_managed, conv_managed, ssm_spec,
conv_spec and the replay resource tuples by the bound prefix length
mamba_num_layers or by the same index/count used when assigning manager-backed
views earlier) so that unmanaged trailing resources remain unmarked and will be
handled by reject_unmanaged_persistent_caches/_allocate_unmanaged_resources.
In `@tests/integration/defs/disaggregated/test_ad_disagg.py`:
- Around line 725-740: The two broad except Exception handlers around
llm.generate_async(...) and the await of futures should be narrowed or
explicitly suppressed: replace the generic catches with concrete exceptions you
expect from the model/runtime (e.g., specific client/runtime errors,
asyncio.CancelledError, and MPI/send errors) or add a local Ruff suppression
comment (e.g., # noqa: BLE001) immediately above each except with a one-line
rationale that these handlers must marshal arbitrary model/runtime failures
across the worker boundary via intercomm.send(worker_error(...), dest=0,
tag=MPI_RESULT); reference the llm.generate_async call site, the futures await
loop, intercomm.send, worker_error, and MPI_RESULT when making the change so the
intent remains clear.
In `@tests/unittest/auto_deploy/singlegpu/shim/test_cached_sequence_interface.py`:
- Around line 1287-1310: The test currently fails earlier due to mismatched
head_dim causing pool-compatibility to trip before the
unmanaged-persistent-cache rejection is exercised; update the
KVPagedResourceHandler construction(s) used by CachedSequenceInterface in
test_initialize_resources_rejects_unmanaged_incompatible_kv so they are
pool-compatible (e.g., make both head_dim values the same — change the second
KVPagedResourceHandler from 80 to 64 to match the first) and keep
reject_unmanaged_persistent_caches=True so initialize_resources() fails on the
unmanaged persistent-cache transfer check as intended (references:
test_initialize_resources_rejects_unmanaged_incompatible_kv,
CachedSequenceInterface.add_resource, KVPagedResourceHandler,
initialize_resources).
🪄 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: 51612b5f-1841-4c16-abd3-a5dd198a244f
📒 Files selected for processing (28)
examples/auto_deploy/model_registry/configs/disagg_ctx.yamlexamples/auto_deploy/model_registry/configs/disagg_gen.yamljenkins/L0_MergeRequest.groovyjenkins/L0_Test.groovytensorrt_llm/_torch/auto_deploy/custom_ops/attention/flashinfer_attention.pytensorrt_llm/_torch/auto_deploy/custom_ops/attention/triton_attention.pytensorrt_llm/_torch/auto_deploy/custom_ops/attention/trtllm_attention.pytensorrt_llm/_torch/auto_deploy/custom_ops/attention_interface.pytensorrt_llm/_torch/auto_deploy/custom_ops/mla/flashinfer_mla.pytensorrt_llm/_torch/auto_deploy/custom_ops/mla/flashinfer_trtllm_mla.pytensorrt_llm/_torch/auto_deploy/custom_ops/mla/trtllm_mla.pytensorrt_llm/_torch/auto_deploy/llm_args.pytensorrt_llm/_torch/auto_deploy/models/custom/modeling_deepseek.pytensorrt_llm/_torch/auto_deploy/shim/ad_executor.pytensorrt_llm/_torch/auto_deploy/shim/interface.pytensorrt_llm/_torch/auto_deploy/transform/library/hidden_states.pytests/integration/defs/disaggregated/test_ad_disagg.pytests/integration/defs/disaggregated/test_ad_disagg_trtllm_serve.pytests/integration/test_lists/qa/llm_function_core.txttests/integration/test_lists/test-db/l0_dgx_h100.ymltests/integration/test_lists/test-db/l0_h100.ymltests/unittest/auto_deploy/singlegpu/custom_ops/test_resource_handlers.pytests/unittest/auto_deploy/singlegpu/shim/test_ad_executor_swa_eviction.pytests/unittest/auto_deploy/singlegpu/shim/test_cached_sequence_interface.pytests/unittest/auto_deploy/singlegpu/shim/test_create_ad_executor.pytests/unittest/auto_deploy/singlegpu/shim/test_llm_config.pytests/unittest/auto_deploy/singlegpu/smoke/test_disagg.pytests/unittest/auto_deploy/singlegpu/transformations/library/test_kv_cache.py
…atch The broad catch(Exception) swallowed InterruptedException, meaning a canceled build could silently fall through and continue scheduling downstream work. Re-throw it first, consistent with the outer getCbtsResult catch block. Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
|
/bot run |
|
PR_Github #52980 [ run ] triggered by Bot. Commit: |
|
PR_Github #52972 [ run ] completed with state |
|
PR_Github #52980 [ run ] completed with state
|
|
/bot skip --comment "skip ci as the failure is not related to the PR, the overall pipeline works fine" |
|
PR_Github #53031 [ skip ] triggered by Bot. Commit: |
|
PR_Github #53031 [ skip ] completed with state |
| def artifactUrl = "${URM_ARTIFACTORY_BASE}/${cbts.cbts_test_db_artifact_path}" | ||
| sh "wget -q '${artifactUrl}' -O /tmp/cbts_test_db.tar.gz && tar xzf /tmp/cbts_test_db.tar.gz -C ${llmSrc}" | ||
| echo "CBTS Layer 3: extracted cbts_test_db from artifact" | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
I think the downloading may always fail when the cross-pipeline reusing happens, since the CBT test-db may only exist in the original pipeline's artifactory path.
|
|
||
| ARTIFACT_PATH = env.artifactPath ? env.artifactPath : "sw-tensorrt-generic/llm-artifacts/${JOB_NAME}/${BUILD_NUMBER}" | ||
| UPLOAD_PATH = env.uploadPath ? env.uploadPath : "sw-tensorrt-generic/llm-artifacts/${JOB_NAME}/${BUILD_NUMBER}" | ||
| URM_ARTIFACTORY_BASE = "https://urm.nvidia.com/artifactory" |
There was a problem hiding this comment.
Could you replace other places with URM_ARTIFACTORY_BASE as well?
The tarball was written to /tmp and uploaded via rtUpload, whose pattern is
resolved relative to the workspace -- so the /tmp path matched 0 files and the
artifact was silently never deployed (uploadArtifacts does not throw on 0
matches). renderTestDB's download then 404'd and fell back to the source
test-db. This was harmless until CBTS split-resize: a resized stage falling
back to the full source list runs it on too few shards and times out. Tar into
and upload from ${LLM_ROOT}, like every other uploadArtifacts call.
Pre-existing since NVIDIA#15142.
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
The tarball was written to /tmp and uploaded via rtUpload, whose pattern is
resolved relative to the workspace -- so the /tmp path matched 0 files and the
artifact was silently never deployed (uploadArtifacts does not throw on 0
matches). renderTestDB's download then 404'd and fell back to the source
test-db. This was harmless until CBTS split-resize: a resized stage falling
back to the full source list runs it on too few shards and times out. Tar into
and upload from ${LLM_ROOT}, like every other uploadArtifacts call.
Pre-existing since NVIDIA#15142.
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
The tarball was written to /tmp and uploaded via rtUpload, whose pattern is
resolved relative to the workspace -- so the /tmp path matched 0 files and the
artifact was silently never deployed (uploadArtifacts does not throw on 0
matches). renderTestDB's download then 404'd and fell back to the source
test-db. This was harmless until CBTS split-resize: a resized stage falling
back to the full source list runs it on too few shards and times out. Tar into
and upload from ${LLM_ROOT}, like every other uploadArtifacts call.
Pre-existing since NVIDIA#15142.
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
The tarball was written to /tmp and uploaded via rtUpload, whose pattern is
resolved relative to the workspace -- so the /tmp path matched 0 files and the
artifact was silently never deployed (uploadArtifacts does not throw on 0
matches). renderTestDB's download then 404'd and fell back to source. Harmless
until CBTS split-resize (resized stage on source list times out). Tar into and
upload from ${LLM_ROOT}, like every other uploadArtifacts call. Pre-existing
since NVIDIA#15142.
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
The tarball was written to /tmp and uploaded via rtUpload, whose pattern is
resolved relative to the workspace -- so the /tmp path matched 0 files and the
artifact was silently never deployed (uploadArtifacts does not throw on 0
matches). renderTestDB's download then 404'd and fell back to source. Harmless
until CBTS split-resize (resized stage on source list times out). Tar into and
upload from ${LLM_ROOT}, like every other uploadArtifacts call. Pre-existing
since NVIDIA#15142.
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
Description
Previously cbts_input_json_b64 piggybacked the raw PR diff (base64-encoded,
up to 256 KB) onto the testFilter Jenkins parameter so each L0_Test agent
could re-run main.py locally to regenerate cbts_test_db/. When diffs were
large this pushed the process environment over Linux ARG_MAX (~2 MB), causing
git to fail with "error=7, Argument list too long" before agents could even
check out the pipeline script.
Fix: the orchestrator now tars the already-generated cbts_test_db/ and
uploads it to Artifactory (${UPLOAD_PATH}/cbts/cbts_test_db.tar.gz) after
main.py runs. Each agent downloads and extracts the tarball directly instead
of re-running main.py. The testFilter parameter no longer carries any large
payload, eliminating the ARG_MAX failure. Layer 3 per-test filtering is fully
preserved; agents fall back to the source test-db if the download fails.
Test Coverage
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.