Skip to content

[https://nvbugs/6627197][fix] Unwaive KV pool rebalance PP loop tests - #18038

Merged
zhaoyuanh-nvidia merged 1 commit into
NVIDIA:mainfrom
zhaoyuanh-nvidia:fix/nvbug-6627197
Aug 24, 2026
Merged

[https://nvbugs/6627197][fix] Unwaive KV pool rebalance PP loop tests#18038
zhaoyuanh-nvidia merged 1 commit into
NVIDIA:mainfrom
zhaoyuanh-nvidia:fix/nvbug-6627197

Conversation

@zhaoyuanh-nvidia

@zhaoyuanh-nvidia zhaoyuanh-nvidia commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Dev Engineer Review

  • Removed four obsolete TestPpLoopDrainWiring NVBug 6627197 waiver entries.
  • Initialized _pending_recompute_pause_ids to an empty set in the PP-loop test fixture.
  • Added CUDA current-stream mocking for CPU-only control-flow tests.
  • The explicit initialization is required because the fixture uses object.__new__.
  • The waiver file change preserves the expected format and scope.
  • No public API or performance changes were identified.
  • git diff --check passes.

QA Engineer Review

  • Modified the PP-loop test fixture in tests/unittest/_torch/executor/test_kv_pool_rebalance.py.
  • Removed four TestPpLoopDrainWiring entries from tests/integration/test_lists/waives.txt.
  • No test functions were added, modified, or removed.
  • The affected tests are no longer waived.
  • The test file reports 57 passing tests.
  • Verdict: sufficient.

Description

The original TestPpLoopDrainWiring failure was fixed by #17874 (5dfe080491da), which initializes _mm_encoder_item_scheduling_enabled in the fixture-created PyExecutor. The four tests nevertheless remained waived under NVBug 6627197.

This PR removes those stale waivers and keeps the fixture compatible with current main by initializing _pending_recompute_pause_ids, which was subsequently introduced by #15252.

The four TestPpLoopDrainWiring tests were waived for NVBug 6627197 because fixture-created PyExecutor instances did not initialize _mm_encoder_item_scheduling_enabled. That initialization has already landed on main, so these waivers are no longer needed.

While validating the tests unwaived on current main, the fixture also needed to initialize _pending_recompute_pause_ids, which was recently added to PyExecutor. The fixture constructs the executor with object.__new__ and therefore must explicitly initialize fields used by the PP loop.

This PR:

  • Removes the four NVBug 6627197 waiver entries.
  • Initializes _pending_recompute_pause_ids in the PP-loop test fixture.

Test Coverage

  • Before the original NVBug fix: 4 tests failed with missing
    _mm_encoder_item_scheduling_enabled.
  • Current main without the fixture update: 54 passed, 3 failed with missing
    _pending_recompute_pause_ids.
  • With this change:
    tests/unittest/_torch/executor/test_kv_pool_rebalance.py — 57 passed.
  • git diff --check passed.

PR Checklist

  • 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.

@zhaoyuanh-nvidia
zhaoyuanh-nvidia requested a review from a team as a code owner August 20, 2026 20:40
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d73b53e7-662e-4f3c-b7e0-74875f7a1b53

📥 Commits

Reviewing files that changed from the base of the PR and between ab78cfe and 8f8fb7a.

📒 Files selected for processing (2)
  • tests/integration/test_lists/waives.txt
  • tests/unittest/_torch/executor/test_kv_pool_rebalance.py

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


Walkthrough

The integration waiver list adds current skipped tests, removes obsolete waivers, and corrects one bug URL. The PP loop test fixture now initializes pending recompute pause IDs and mocks CUDA stream access for CPU-based tests.

Changes

Test alignment updates

Layer / File(s) Summary
Integration waiver list updates
tests/integration/test_lists/waives.txt
The waiver list adds skips for disaggregated serving, model accuracy, KV-cache scheduling, performance, and visual-generation tests. It removes obsolete entries and corrects one malformed bug URL.
PP loop fixture alignment
tests/unittest/_torch/executor/test_kv_pool_rebalance.py
The fixture initializes _pending_recompute_pause_ids and mocks torch.cuda.current_stream for CPU-based PP loop tests.

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

Merge Risk: 🟡 Moderate · up to 8f8fb

This change updates the PP-loop test fixture and removes four stale waivers, with 57 targeted tests passing. Merge readiness remains gated by a completed successful CI run because the available pipeline results include failures and no completed rerun.

Suggested reviewers: juney-nvidia, eopxd, bowenfu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the repository format and clearly identifies the main change: removing waivers for KV pool rebalance PP loop tests.
Description check ✅ Passed The description explains the issue, solution, affected tests, test results, and checklist status with sufficient detail.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@zhaoyuanh-nvidia

Copy link
Copy Markdown
Collaborator Author

/bot run --extra-stage "CPU-Generic-x86-1"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #67960 [ run ] triggered by Bot. Commit: ab78cfe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #67960 [ run ] completed with state SUCCESS. Commit: ab78cfe
/LLM/main/L0_MergeRequest_PR pipeline #55413 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@zhaoyuanh-nvidia

Copy link
Copy Markdown
Collaborator Author

/bot run --extra-stage "CPU-Generic-x86-1"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #68068 [ run ] triggered by Bot. Commit: ab78cfe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #68068 [ run ] completed with state SUCCESS. Commit: ab78cfe
/LLM/main/L0_MergeRequest_PR pipeline #55516 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@zhaoyuanh-nvidia

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --extra-stage "CPU-Generic-x86-1"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #68345 [ run ] triggered by Bot. Commit: ab78cfe Link to invocation

Signed-off-by: Zhaoyuan He <zhaoyuanh@nvidia.com>
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #68345 [ run ] completed with state ABORTED. Commit: ab78cfe

Link to invocation

@zhaoyuanh-nvidia

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --extra-stage "CPU-Generic-x86-1"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #68537 [ run ] triggered by Bot. Commit: 8f8fb7a Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #68537 [ run ] completed with state SUCCESS. Commit: 8f8fb7a
/LLM/main/L0_MergeRequest_PR pipeline #55952 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast --extra-stage "CPU-Generic-x86-1"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #68583 [ run ] triggered by Bot. Commit: 8f8fb7a Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #68583 [ run ] completed with state SUCCESS. Commit: 8f8fb7a
/LLM/main/L0_MergeRequest_PR pipeline #55995 completed with status: 'SUCCESS'

CI Report

Link to invocation

@zhaoyuanh-nvidia
zhaoyuanh-nvidia enabled auto-merge (squash) August 23, 2026 23:36
@zhaoyuanh-nvidia
zhaoyuanh-nvidia merged commit 0db09f5 into NVIDIA:main Aug 24, 2026
11 checks passed
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.

5 participants