[None][infra] CBTS code coverage date early save - #17253
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughCBTS coverage eligibility now uses ChangesCBTS coverage pipeline
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Pipeline
participant AlpinePod
participant ResultStorage
Pipeline->>AlpinePod: Run uploadArchCoverage
AlpinePod->>ResultStorage: Download single-GPU result archives using UPLOAD_PATH
AlpinePod->>AlpinePod: Extract and merge SQLite coverage files
AlpinePod->>ResultStorage: Upload compressed architecture touch database
Pipeline->>Pipeline: Start multi-GPU testing
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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.
Actionable comments posted: 3
🤖 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 1618-1619: Move the x86_64 uploadArchCoverage call in
jenkins/L0_MergeRequest.groovy at lines 1618-1619 immediately after the x86_64
single-GPU stage and before the requireMultiGpuTesting early return; likewise
move the SBSA uploadArchCoverage call at lines 1736-1737 immediately after the
SBSA single-GPU stage and before its gate, preserving both existing calls and
arguments.
- Line 1211: Update the testResultLink construction in launchJob to use the
configured UPLOAD_PATH value rather than the hardcoded Artifactory path,
preserving the existing JOB_NAME and BUILD_NUMBER segments before appending
/test-results so overridden upload paths resolve the downloaded archives
correctly.
- Around line 1215-1219: Update the coverage download and extraction flow around
trtllm_utils.llmExecStepWithRetry and the results-*.tar.gz find command to
isolate artifacts for the requested architecture. Use architecture-specific
result directories, or filter downloads so only the requested single-GPU archive
is extracted, ensuring coverage from the other architecture cannot be merged and
duplicate SQLite basenames cannot overwrite each other.
🪄 Autofix
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: 481b671a-3656-4d2b-8da0-2a7608073b0a
📒 Files selected for processing (1)
jenkins/L0_MergeRequest.groovy
92cf856 to
1e75000
Compare
|
/bot run --disable-fail-fast --stage-list "A10-PyTorch-1, GB10-PyTorch-1" |
|
PR_Github #63755 [ run ] triggered by Bot. Commit: |
|
/bot kill |
1e75000 to
803080f
Compare
|
PR_Github #63768 [ kill ] triggered by Bot. Commit: |
|
PR_Github #63755 [ run ] completed with state |
|
PR_Github #63768 [ kill ] completed with state |
|
/bot run --disable-fail-fast --stage-list "A10-PyTorch-1, A30-PyTorch-1, GB300-PyTorch-1, GB10-PyTorch-1" |
|
PR_Github #63771 [ run ] triggered by Bot. Commit: |
|
PR_Github #63771 [ run ] completed with state |
|
Could you tighten the rollout and failure handling before merge?
|
|
/bot run --disable-fail-fast --stage-list "A10-PyTorch-1, A30-PyTorch-1, GB300-PyTorch-1, GB10-PyTorch-1" |
|
PR_Github #63906 [ run ] triggered by Bot. Commit: |
|
The arch filter in
None of this is a correctness objection to the upload helper itself, which reads fine. |
|
PR_Github #63906 [ run ] completed with state |
|
/bot run |
|
PR_Github #63951 [ run ] triggered by Bot. Commit: |
|
PR_Github #63951 [ run ] completed with state
|
|
/bot skip --comment "the failure is not related to this pr" |
|
PR_Github #64011 [ skip ] triggered by Bot. Commit: |
|
PR_Github #64011 [ skip ] completed with state |
|
PR_Github #65029 [ run ] triggered by Bot. Commit: |
|
PR_Github #65029 [ run ] completed with state
|
2bdd0b4 to
ce47502
Compare
|
/bot skip --comment "skip ci as only do low risk rebase" |
|
PR_Github #65207 [ skip ] triggered by Bot. Commit: |
|
PR_Github #65207 [ skip ] completed with state |
niukuo
left a comment
niukuo
left a comment
There was a problem hiding this comment.
Approved with the non-blocking helper-timeout follow-up noted inline.
|
/bot run --stage-list "" |
|
PR_Github #65476 [ run ] triggered by Bot. Commit: |
|
PR_Github #65476 [ run ] completed with state |
|
/bot run |
|
PR_Github #65505 [ run ] triggered by Bot. Commit: |
|
PR_Github #65505 [ run ] completed with state |
…pipeline
In official post-merge runs, single-GPU coverage data is ready well before
multi-GPU stages finish, but the selector had to wait for the full pipeline
to complete before any coverage artifact appeared on Artifactory.
Add uploadArchCoverage(arch) called immediately after each arch's single-GPU
job (before multi-GPU starts). It spins up a lightweight alpine K8s pod,
downloads the already-uploaded single-GPU result tarballs, merges the
.cbtscov.*.sqlite files via pystart_report.py (sqlite only, no HTML), and
uploads cbts_pystart_report_{arch}.tar.gz to cbts-coverage/.
Update artifact.py to prefer these early per-arch artifacts when the selector
fetches the touch DB: if either cbts_pystart_report_x86_64.tar.gz or
cbts_pystart_report_SBSA.tar.gz exists for the latest build, their sqlite
files are merged and returned immediately without waiting for the complete
cbts_pystart_report.tar.gz (which collectTestResults still uploads at the
end with full single+multi-GPU data and HTML report).
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
…load verification Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
…sult collection Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
…ent cross-contamination x86_64 and SBSA jobs upload to the same test-results directory. Without filtering, each arch's early coverage sqlite ends up containing data from both architectures. Filter the download list by known SBSA stage name prefixes (GH200-, GB10-, GB200-, GB300-, CPU-Generic-arm-) so each report only contains its own arch's coverage data. Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
…s for upload verification" This reverts commit 803080f. Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
…ent cross-contamination x86_64 and SBSA jobs upload to the same test-results directory. Without filtering, each arch's early coverage sqlite ends up containing data from both architectures. Filter the download list by known SBSA stage name prefixes (GH200-, GB10-, GB200-, GB300-, CPU-Generic-arm-) so each report only contains its own arch's coverage data. Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
Broad Exception catch was swallowing Jenkins pipeline cancellations and user aborts (FlowInterruptedException / InterruptedException), causing the helper to log them as non-fatal and continue. Rethrow to match the pattern used by other best-effort blocks in this file. Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
…rchCoverage The helper-specific timeout(15, MINUTES) raises FlowInterruptedException, which the broad InterruptedException handler rethrew, making the uploader timeout fail the entire pipeline against the non-fatal contract of this helper. Catch FlowInterruptedException and treat only an ExceededTimeout cause as non-fatal, continuing to rethrow user aborts and upstream pipeline interruptions. Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
72706b4 to
709f83a
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #65509 [ run ] triggered by Bot. Commit: |
|
PR_Github #65509 [ run ] completed with state
|
|
/bot skip --comment "skip ci as the failure is not related to this pr" |
|
PR_Github #65753 [ skip ] triggered by Bot. Commit: |
|
PR_Github #65753 [ skip ] completed with state |
4 participants
Dev Engineer Review
uploadArchCoverage(String arch, pipeline, testFilter)injenkins/L0_MergeRequest.groovy.cbts-coverage/.${UPLOAD_PATH}.PostMergejobs. Limit this scope before merge.wgetlistings, timeout or interruption exceptions, extraction failures, merge failures, and upload failures remain best-effort.QA Engineer Review
No test changes.
Description
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.