[https://nvbugs/6462303][chore] Unwaive KV cache v2 scheduler tests with executor loop tracing - #17039
Conversation
…ith executor loop tracing test_chunked_prefill (nvbugs 6428002) and test_eviction_with_block_reuse (nvbugs 6462303) are waived as flaky. Both only ever report "Test terminated unexpectedly" in CI, which gives no indication of where the session died, and 50 full-stage reruns of DGX_B200-PyTorch on current main did not reproduce either failure locally. Unwaive both and opt them into TLLM_TRACE_EXECUTOR_LOOP so the next CI occurrence records exactly where the executor loop stopped. Tracing is applied through a fixture on these two tests only, since the line trace is expensive (~1.9M stdout lines / ~157 MB per traced test) -- monkeypatch restores the environment afterwards, and the MPI pool cache already retires pools whose spawn environment no longer matches, so no other test in the session is traced. Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughTwo KV cache scheduler integration tests now enable executor-loop tracing through a scoped pytest fixture. Their skip entries are removed from the integration waive list. ChangesKV cache scheduler tracing
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run --disable-fail-fast --stage-list "DGX_B200-PyTorch-1, DGX_B200-PyTorch-8" |
|
PR_Github #62661 [ run ] triggered by Bot. Commit: |
BowenFu
left a comment
BowenFu
left a comment
There was a problem hiding this comment.
Checked the isolation, since both tests re-enter pre-merge B200 (l0_b200.yml:3-16, lines 165/175): _spawn_snapshot() compares the whole environment minus a short ignore list, so setting and then unsetting TLLM_TRACE_EXECUTOR_LOOP retires the cached MPI pool on both edges — the variable can't reach a neighbouring test, and mpi_session.py forwards TLLM* at spawn so it does reach the worker. Waiver removal is exact-ID; test_chunked_prefill_multi_request and test_chunked_prefill_with_eviction are untouched.
One caveat I'm not blocking on: trace_func uses unflushed print(), so an abrupt kill can lose the final user-space buffer — you'll get essentially all 157 MB but not a guaranteed last line. If the next occurrence lands and the tail is ambiguous, that's why.
Worth stating for whoever picks this up later: this is the diagnostics step Po-Han asked for on both 6428002 and 6462303 ("run 50 times / file a PR to dump more debugging info"), so the tracing is temporary by construction — please do drop it once either bug is root-caused.
|
PR_Github #62661 [ run ] completed with state
|
fredricz-20070104
left a comment
fredricz-20070104
left a comment
There was a problem hiding this comment.
Review summary - Approve
Reviewed the full diff; no blocking or major issues found.
Minor, non-blocking notes:
tests/integration/test_lists/waives.txt: Re-enabling a pre-merge flaky test can block unrelated PRstests/integration/defs/kv_cache/test_kv_cache_v2_scheduler.py: Tracing is temporary by construction — leave a removal marker
Automated review by NVCortex Lite, run by @fredricz-20070104.
4 participants
Summary
Unwaives the two flaky KV cache v2 scheduler tests and instruments them so the next CI occurrence is actually diagnosable.
TestKVCacheV2Llama::test_chunked_prefillTest timeout/Test terminated unexpectedly(DGX_B200-PyTorch, post-merge)TestKVCacheV2Llama::test_eviction_with_block_reuseTest terminated unexpectedly(DGX_B200-PyTorch, pre-merge)Both bugs only ever record
Test terminated unexpectedly— the pytest session dies with no indication of where, so there is nothing to debug from the CI artifacts.Why unwaive now
I reran the full
l0_b200pre-merge/pytorch shards containing each test (9-wayleast_durationsplit, same pytest command line asjenkins/L0_Test.groovy) 50x each on B200. The target tests passed in every completed iteration — the failure does not reproduce on demand, so waiting for a local repro is not a viable path. Getting a trace out of the next CI occurrence is.Tracing scope
TLLM_TRACE_EXECUTOR_LOOP=ALLmakestrace_func(tensorrt_llm/_utils.py) print every executed line of the PyExecutor event loop, so the last line before the process dies pinpoints the hang.The trace is expensive, so it is opted into via a fixture on these two tests only — never module- or stage-wide. Verified experimentally by running the traced test alongside three untraced ones in a single session:
monkeypatchrestores the environment after the test, and the MPI pool cache independently refuses to reuse a pool whose spawn environment changed ([session-reuse] retiring cached pool: env changed since spawn: ['TLLM_TRACE_EXECUTOR_LOOP']), so the variable cannot leak into a reused worker.Measured overhead on the traced test: ~32s -> ~40s.
Note the stage log grows by roughly 150 MB per traced test. The tracing is meant to be temporary — it should be removed once either bug is root-caused.
Test Coverage
kv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2Llama::test_chunked_prefill,kv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2Llama::test_eviction_with_block_reusePR Checklist
[fix],[feat],[doc],[infra],[chore]🤖 Generated with Claude Code
Summary by CodeRabbit