Skip to content

[https://nvbugs/6902353][chore] Unwaive transfer lifecycle GPU tests - #20439

Open
chienchunhung wants to merge 1 commit into
NVIDIA:mainfrom
chienchunhung:dev/nvbug-6902353-unwaive
Open

chienchunhung wants to merge 1 commit into
NVIDIA:mainfrom
chienchunhung:dev/nvbug-6902353-unwaive

Conversation

@chienchunhung

@chienchunhung chienchunhung commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Restore CI coverage for the six transfer-lifecycle GPU cases waived under NVBUG 6902353. Repeated remote-cluster verification did not reproduce the worker-startup timeout with the original 120-second setting, supporting re-enabling these cases for validation in CI.

Scope

Remove only the six case-specific entries in waives.txt: baseline, cancel_before_publication, cancel_late_aux, cancel_late_kv, source_fatal, and timeout_late_aux.
The existing DGX B200 test-list entry is retained. Upstream subsequently added a destination_fatal waiver in #20457; this rebase preserves that waiver, so this PR re-enables only the original six cases.
The test harness, assertions, 120-second startup timeout, 600-second pytest timeout, and product code are unchanged.

Verification

Local checks at c3327dd51466ee52487c4e3709dde534e908fb18: pre-commit run --files tests/integration/test_lists/waives.txt and git diff --check passed, including waiver sorting/duplication, formatting, and AST test-list validation; the original DCO sign-off is retained. Rebasing onto df4b424b0028a8838effbc4313c6411fb36d299d resolved one waiver-list conflict by preserving upstream's new destination_fatal waiver and removing the same six entries. An independent CPU check executed the repository's CBTS WaivesRule on the base-to-head diff and selected the existing four-GPU MPI lifecycle-test block and exactly the three DGX B200 PyTorch stages below.

Historical remote-cluster evidence, October 15, 2026 — four B200 GPUs:

Evidence Result
Requested coverage Seven cases × ten repetitions; 70 unique executions, no replays
Reconciled results 68 JUnit/guard-validated passes
Incomplete final evidence Two additional completed passing case-guard records; final JUnit was lost to ENOSPC
Observed test failures / skips / test errors 0 / 0 / 0
Startup-120s / MPI-exit-60s / pytest-600s failures 0 / 0 / 0

Slurm job 5023705 produced 21 validated passes and the two guard-only passes before ENOSPC interrupted result publication. Job 5025564 executed only the 47 never-started identities: all 47 passed, and the job completed with exit 0:0. Repetition 4 was split across attempts; this is not a claim of ten uninterrupted pytest invocations or 70 fully validated passes. All 70 case-process cleanup guards settled without intervention.

The actual pytest invocations used python3 -m pytest -p lifecycle_guard -vv -ra --tb=long -o junit_family=xunit2, explicit lifecycle node IDs, and per-repetition JUnit/temporary directories. The original harness at dc8451c50864edc07c75dedeafcb91dbb7a5d478 is byte-identical to this PR's harness (SHA256 c51885ea28f18e879f8cbc1477bd9c7c26317887d340abff1715d39013d49409); installed native runtime was da676ffb8b611a1a0a83c862aa094e736f9fe5aa, version 1.4.0rc1.dev202610140007. This newer runtime and environment differ from the historical failing CI: intervening code, dependency, or environment changes may explain the difference, but no causal fix has been isolated.

Rebased-head GPU CI has not been triggered by this workflow. The multi-GPU stages require ci: full pre-merge approved, granted through the review/CI-approver workflow. At the post-rebase check, the required approval label was absent and no Jenkins pipeline had started; automatic GitHub repository checks were running. After approval, validate the current DGX_B200-4_GPUs-PyTorch-1/2/3 stage family (sharding can move the test), confirming that the six re-enabled cases execute; destination_fatal remains waived by #20457. The offline results are supporting evidence, not an exact original-CI replay or a CI pass on this PR's native build; full pre-merge CI remains required separately from a stage-only run.

Notes

Reverses the six waivers introduced by #20235. A subsequent reviewer report describes failures in whichever lifecycle case launches first in CI and proposes cold-start overhead as an explanation; that reported CI history has not been independently reproduced by the offline experiment. Rebase alone does not address that concern. Keep the bug open pending resolution and required CI validation; this PR alone also leaves the #20457 waiver in place.

  • Repository PR checklist reviewed as applicable: this change restores existing test coverage and adds no product behavior, API changes, dependencies, ownership changes, or documentation requirements.

Dev Engineer Review

The change removes six GPU transfer-lifecycle test waivers. It does not change product code or test behavior. The destination_fatal case remains enabled. Full CI validation remains a follow-up: PR-head GPU CI is pending approval, and the reported remote-cluster runs used a newer runtime and environment. No causal fix has been isolated. Current review-finding counts are unavailable.

QA Engineer Review

tests/integration/test_lists/waives.txt removes waivers for baseline, cancel_before_publication, cancel_late_aux, cancel_late_kv, source_fatal, and timeout_late_aux. The test harness is unchanged. The DGX B200 CI list still includes unittest/disaggregated/test_transfer_lifecycle_gpu.py. Reported historical runs covered all seven cases across ten repetitions, but they do not establish a pass on this PR’s native build. Coverage verdict: needs follow-up until full pre-merge CI runs without waivers.

Per-File QA Perspective

  • tests/integration/test_lists/waives.txt: Removing the six entries allows the corresponding GPU tests to run instead of being waived. The test module remains listed in tests/integration/test_lists/test-db/l0_dgx_b200.yml; verify all seven cases in full CI.

@chienchunhung
chienchunhung marked this pull request as ready for review October 15, 2026 21:10
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/TensorRT-LLM/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 362e7144-d597-4d04-adcb-5ba6e7f4a4a6
📥 Commits

Reviewing files that changed from the base of the PR and between ab6f35d and c3327dd.

📒 Files selected for processing: 1
  • tests/integration/test_lists/waives.txt
💤 Files with no reviewable changes: 1
  • tests/integration/test_lists/waives.txt

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The integration waiver list retains the destination_fatal skip entry. It no longer includes skip entries for six other GPU transfer lifecycle cases.

Changes

GPU transfer lifecycle tests

Layer / File(s) Summary
Remove GPU transfer test waivers
tests/integration/test_lists/waives.txt
Removed skip entries for baseline, both cancellation cases, source_fatal, and timeout_late_aux. The destination_fatal entry remains.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Other

Suggested reviewers: bowenfu

Merge Risk: ⚪ Minimal · up to c3327

No confirmed issue prevents merging. The required pre-merge GPU CI should run to validate the restored tests on this PR’s build.

Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the NVBugs ticket, change type, and primary change: re-enabling the transfer lifecycle GPU tests.
Description check Passed The description explains the change, scope, rationale, verification results, limitations, pending CI validation, and checklist status. It does not use the template's exact "Description" and "Test Cove…
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.

Full details: Description check

Explanation

The description explains the change, scope, rationale, verification results, limitations, pending CI validation, and checklist status. It does not use the template's exact "Description" and "Test Coverage" headings, but it provides the required information in equivalent sections.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@lowsfer

lowsfer commented Oct 16, 2026

Copy link
Copy Markdown
Member

Heads-up before this merges: in CI, the worker-startup timeout tracked by NVBUG 6902353 consistently hits whichever test_transfer_lifecycle_gpu case runs first in DGX_B200-4_GPUs-PyTorch-3. Unwaiving these six is likely to move the failure back to baseline rather than clear it.

Evidence from x86-64 Multi-GPU runs:

That fits a cold-start cost on a fresh CI node: the first launch exceeds the 120 s startup deadline and later launches are warm. It would also explain why repeated runs on a warm remote cluster don't reproduce it.

#20457 waives destination_fatal under this same bug to unblock pre-merge in the meantime. Suggest holding this unwaive until the first-launch startup is fixed, e.g. a longer deadline for the first launch or a per-module warmup. Otherwise baseline will likely start failing every Multi-GPU run again. This PR's own CI may not catch it: a waive-only change typically doesn't exercise the x86-64 Multi-GPU leg.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
@chienchunhung
chienchunhung force-pushed the dev/nvbug-6902353-unwaive branch from ab6f35d to c3327dd Compare October 16, 2026 20:03
@chienchunhung

Copy link
Copy Markdown
Collaborator Author

Heads-up before this merges: in CI, the worker-startup timeout tracked by NVBUG 6902353 consistently hits whichever test_transfer_lifecycle_gpu case runs first in DGX_B200-4_GPUs-PyTorch-3. Unwaiving these six is likely to move the failure back to baseline rather than clear it.

Evidence from x86-64 Multi-GPU runs:

That fits a cold-start cost on a fresh CI node: the first launch exceeds the 120 s startup deadline and later launches are warm. It would also explain why repeated runs on a warm remote cluster don't reproduce it.

#20457 waives destination_fatal under this same bug to unblock pre-merge in the meantime. Suggest holding this unwaive until the first-launch startup is fixed, e.g. a longer deadline for the first launch or a per-module warmup. Otherwise baseline will likely start failing every Multi-GPU run again. This PR's own CI may not catch it: a waive-only change typically doesn't exercise the x86-64 Multi-GPU leg.

Thanks for the notice @lowsfer . Can you provide the reference PR that this PR should wait for?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants