Skip to content

[None][fix] Fix gen-only benchmark for KVCacheManager V2 + improve insufficient KVCache check - #13625

Closed
Tabrizian wants to merge 13 commits into
NVIDIA:feat/deepseek_v4from
Tabrizian:user/imant/dsv4-bench-v2-watchdog
Closed

[None][fix] Fix gen-only benchmark for KVCacheManager V2 + improve insufficient KVCache check#13625
Tabrizian wants to merge 13 commits into
NVIDIA:feat/deepseek_v4from
Tabrizian:user/imant/dsv4-bench-v2-watchdog

Conversation

@Tabrizian

Copy link
Copy Markdown
Member

Description

Two related fixes for the gen-only disagg benchmark mode on KVCacheManagerV2.

1. KV-cache capacity drift in the bench fill phase

In gen-only benchmark mode the executor calls _check_benchmark_disagg_gate every iteration and continues when the fill phase is not yet complete. Before that, the scheduler has already called try_allocate_generation, which grows each gen request's KV capacity by 1 + draft_len. Without a matching revert the capacity drifts upward across the many retried iterations until it overflows the host page-index buffer (sized by max_seq_len/tokens_per_block rounded up to a multiple of 4), eventually raising:

ValueError: User-provided base page indices is too short

from KVCacheManagerV2._KVCache.resize.

Fix: revert the spurious growth on the should_retry path in both _executor_loop and _executor_loop_overlap, gated by _scheduler_manages_kv_suspend so V1 is unaffected.

2. Insufficient-KV guard never fires under attention DP

The previous "Insufficient KV cache for gen-only benchmark mode" guard compared the per-rank num_fetch_requests against the global benchmark_req_queues_size threshold. Under attention DP, prompts are routed across TP ranks via the ADP router, so per-rank fetch counts saturate well below the global threshold and the guard never fires — deadlocks become silent walltime hangs (e.g. KV pool too small to fit threshold, transceiver wedged).

Fix: replace the guard with a liveness watchdog that tracks the per-rank ready-to-forward gen request count (DISAGG_GENERATION_TRANS_COMPLETE + GENERATION_IN_PROGRESS); if the count does not change for >60s, log an explicit error, fail all active requests via _handle_errors, and return None to break the executor loop. Local-only count to avoid divergent collectives across ranks (ADP can route different batches per rank).

Test Coverage

Verified end-to-end on Lyris (2-node disagg, ctx tp=1 ep=1 × 2 servers, gen tp=2 ep=2 attn_dp=on, max_seq_len=2088, max_batch_size=1024):

  • Without the revert fix the run reproducibly hits ValueError: User-provided base page indices is too short at gen iter ~1000.
  • With the revert fix in place, the existing reproducer no longer drifts (max kv_cache.capacity stays bounded near prompt_len + 1).
  • The watchdog correctly detects the underlying KV-pool-too-small condition that previously hung silently and aborts within 60s with: Benchmark gen request count stalled at … (per-rank, ready-to-forward) for 60s … Aborting all active requests.

Existing tests/unittest/_torch/executor/test_benchmark_disagg.py covers the gate logic; no new test added — the revert is a 3-line predicate and the watchdog is timing-dependent (would require fault injection to unit test deterministically).

PR Checklist

  • PR description clearly explains what and why.

  • PR follows the TRT-LLM CODING GUIDELINES.

  • Test cases provided for new code paths — existing benchmark tests cover the gate path; the watchdog timing is hard to unit-test.

  • Any new dependencies have been scanned for license and vulnerabilities — none added.

  • CODEOWNERS updated if ownership changes — no ownership change.

  • Documentation updated as needed — none required.

  • Update tava architecture diagram if there is a significant design change in PR — no architectural change.

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

lfr-0531 and others added 12 commits April 28, 2026 12:26
Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com>
…VIDIA#13563)

Signed-off-by: Shixiaowei02 <39303645+Shixiaowei02@users.noreply.github.com>
Co-authored-by: Chuang Zhu <111838961+chuangz0@users.noreply.github.com>
Signed-off-by: Chuang Zhu <111838961+chuangz0@users.noreply.github.com>
…VIDIA#13564)

Signed-off-by: Lanyu Liao <lancelly@users.noreply.github.com>
….llm_args (NVIDIA#13568)

Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
Co-authored-by: Yuewei Na <nv-yna@users.noreply.github.com>
Signed-off-by: Mingyang Hao <mingyangHao@users.noreply.github.com>
Signed-off-by: Jiagan Cheng <jiaganc@nvidia.com>
Signed-off-by: Mingyang Hao <mingyangHao@users.noreply.github.com>
Co-authored-by: Mingyang Hao <mingyangHao@users.noreply.github.com>
Signed-off-by: Barry Kang <43644113+Barry-Delaney@users.noreply.github.com>
Signed-off-by: Tracin <10434017+Tracin@users.noreply.github.com>
Signed-off-by: Qi Zhang (qizh) <10434017+Tracin@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
…king fill loop (NVIDIA#12208)

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
…sufficient KVCache check

In gen-only benchmark mode the executor calls
_check_benchmark_disagg_gate every iteration and `continue`s when the
fill phase is not yet complete.  Before that the scheduler has already
called try_allocate_generation, which grows each gen request's KV
capacity by 1 (+draft_len).  Without a matching revert the capacity
drifts upward across the many retried iterations until it overflows
the host page-index buffer, raising

  ValueError: User-provided base page indices is too short

from KVCacheManagerV2._KVCache.resize.  Revert the spurious growth on
the should_retry path in both _executor_loop and _executor_loop_overlap,
gated by _scheduler_manages_kv_suspend so V1 is unaffected.

The previous "Insufficient KV cache for gen-only benchmark mode" guard
compared the per-rank num_fetch_requests against the global
benchmark_req_queues_size threshold.  Under attention DP, prompts are
routed across TP ranks via the ADP router, so per-rank fetch counts
saturate well below the global threshold and the guard never fires --
deadlocks become silent walltime hangs.  Replace it with a liveness
watchdog that tracks the per-rank ready-to-forward gen request count
(DISAGG_GENERATION_TRANS_COMPLETE + GENERATION_IN_PROGRESS); if the
count does not change for >60s, log an explicit error, fail all
active requests via _handle_errors, and return None to break the
executor loop.  The watchdog uses a local-only count so it does not
introduce a collective at a point where rank participation can diverge
under ADP.

Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
@Tabrizian
Tabrizian requested a review from a team as a code owner April 29, 2026 23:39
@Tabrizian
Tabrizian requested review from lancelly and removed request for a team April 29, 2026 23:39
@Tabrizian

Copy link
Copy Markdown
Member Author

/bot run --disable-fail-fast

@Tabrizian
Tabrizian requested a review from qiaoxj07 April 29, 2026 23:43
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #46225 [ run ] triggered by Bot. Commit: 0f51dd5 Link to invocation

Signed-off-by: Chuang Zhu <111838961+chuangz0@users.noreply.github.com>
@Tabrizian
Tabrizian requested review from a team as code owners April 30, 2026 01:04
@Tabrizian
Tabrizian requested review from byshiue and hyukn and removed request for a team April 30, 2026 01:04
@nvpohanh

Copy link
Copy Markdown
Collaborator

@yizhang-nv could you review this? thanks

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #46225 [ run ] completed with state FAILURE. Commit: 0f51dd5
/LLM/main/L0_MergeRequest_PR pipeline #36337 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

Link to invocation

@lfr-0531
lfr-0531 force-pushed the feat/deepseek_v4 branch from 67b0b17 to 9c57516 Compare May 7, 2026 16:30
@lfr-0531
lfr-0531 requested review from a team as code owners May 7, 2026 16:30
@lfr-0531
lfr-0531 requested review from a team as code owners May 7, 2026 16:30
@lfr-0531
lfr-0531 requested a review from a team May 7, 2026 16:30
@lfr-0531
lfr-0531 requested review from a team as code owners May 7, 2026 16:30
@Tabrizian Tabrizian closed this May 7, 2026
@Tabrizian

Copy link
Copy Markdown
Member Author

Closing It was included in #13650 PR.

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.