[None][chore] Replace cache_salt_id impl with main's TreeTaskId (cp #13793) by lancelly · Pull Request #14351 · NVIDIA/TensorRT-LLM

lancelly · 2026-05-20T09:00:04Z

Summary

Replaces feat/deepseek_v4's parallel cache_salt_id impl (introduced as a side effect of PR #13624 / commit 07adb42, which mainly added V1↔V2 KV event hash matching) with main's TreeTaskId abstraction from PR #13793.

Both implementations achieve identical user-visible behavior (per-request salt-based KV cache reuse isolation). The differences are internal:

	DSv4 (before)	this PR (after)
Radix tree	knows `lora_task_id` + `cache_salt_id` as 2 fields	opaque `tree_task_id: int \| bytes \| None`
Hash mixing	inside the tree, via `_make_root_key` + tag bytes	above the tree, in `_KVCache._make_tree_task_id`
`RootBlock`	stores both fields	stores single `tree_task_id`

The opaque-seed shape allows mixing future dimensions (e.g. multi-modal digest) without changing the tree. It is also the prerequisite for cherry-picking PR #14140 (ReuseScope NamedTuple refactor) and PR #14333 (KV cache reuse probe), which will follow in subsequent PRs.

Conflict resolution notes

Dummy/warmup requests: adopted main's design — no salt on dummies. Rationale: dummy paths stop_committing() anyway so the salt choice is inert, but explicitly passing None avoids coupling synthetic data to any salted branch.
IndexMapper saturation guard: kept DSv4's None-return path in _create_kv_cache (needed for disagg gen-trans-in-progress requests).
Event manager / try_get_tree / other DSv4 hooks: preserved verbatim in the touched files — git auto-merge handled these because [None][feat] Support cache_salt_id in KV cache v2 manager #13793 does not touch the same lines.

lancelly · 2026-05-20T09:14:09Z

@peihu-nv and @eopXD Could you please take a look at this PR? Currently the salt implementation seems to diverge on feat/deepseek_v4 and main branch. To cherry-pick other kv manager related code onto feat/deepseek_v4, we need to resolve this conflict first.

Signed-off-by: Yueh-Ting Chen <yueh.ting.chen@gmail.com> (cherry picked from commit 4baeec3) Signed-off-by: lancelly <108499334+lancelly@users.noreply.github.com>

lancelly · 2026-05-20T15:15:11Z

/bot run --disable-fail-fast

peihu-nv

Direction LGTM, approving to unblock the stack assuming follow up PR to land right after.

Seems to me that test_v2_root_key_distinguishes_lora_from_cache_salt_id still calls RootBlock.make_key(None, None), now it is a single arg.
_event_manager._root_attrs_from_root_block still reads lora_task_id/cache_salt_id off RootBlock, but this PR removes both → silently (None, None) for every event → V1-compat hashes break for any LoRA/salt request and Dynamo routing degrades. Not sure if dropping V1 compat is intentional or might need to address this in the ReuseScope follow-up (root.reuse_scope.lora_id/.salt).

peihu-nv · 2026-05-20T18:10:01Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-05-20T18:17:13Z

PR_Github #49467 [ run ] triggered by Bot. Commit: e457809 Link to invocation

tensorrt-cicd · 2026-05-20T21:56:49Z

PR_Github #49467 [ run ] completed with state SUCCESS. Commit: e457809
/LLM/main/L0_MergeRequest_PR pipeline #39108 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

After NVIDIA#14140 (cherry-picked in NVIDIA#14353) RootBlock no longer exposes ``lora_task_id`` / ``cache_salt_id`` directly — both fields are folded into a ``ReuseScope`` NamedTuple attached as ``root.reuse_scope``. ``KVCacheEventManager._root_attrs_from_root_block`` was still reading the legacy attribute names via ``getattr(..., None)``, so every emitted event silently received ``(None, None)`` and the V1-compat hash collapsed to the same value for any LoRA/salt request. Downstream Dynamo routing depends on those hashes, so this regression would materially degrade prefix cache hit rate for any request carrying a LoRA task id or a cache salt. Fix ``_root_attrs_from_root_block`` to prefer ``root.reuse_scope`` and fall back to the legacy attributes (keeps any in-flight pre-refactor RootBlock instances working). Also: - Update the now-stale ``test_v2_root_key_distinguishes_lora_from_cache_salt_id`` to use the new single-arg ``RootBlock.make_key(ReuseScope(...))`` API. - Extend ``_FakeRootBlock`` with an optional ``reuse_scope`` kwarg so tests can mimic both the pre- and post-refactor RootBlock shape. - Add a regression test ``test_v2_kv_cache_event_manager_v1_hash_reads_root_reuse_scope`` that asserts (a) a ReuseScope-shaped root and a legacy root with the same lora/salt produce identical V1 event hashes, (b) different scopes still produce different hashes (no silent collapse), and (c) an empty ReuseScope matches a legacy root with no salt/lora. Addresses reviewer feedback on NVIDIA#14351 (peihu-nv). Signed-off-by: lancelly <108499334+lancelly@users.noreply.github.com>

Build #39108 of PR NVIDIA#14351 failed 7 tests in tests/unittest/disaggregated/ test_router.py with: TypeError: RootBlock.make_key() takes 1 positional argument but 2 were given This PR collapses the prior (lora_task_id, cache_salt_id) pair into a single TreeTaskId seed for the radix tree, but two callers on the base feat/deepseek_v4 branch were not migrated: - tensorrt_llm/serve/router.py — two call sites still pass (None, cache_salt_id) to V2RootBlock.make_key - tests/unittest/disaggregated/test_router.py — two call sites still pass cache_salt_id as the 4th positional argument to sequence_to_blockchain_keys Fix by computing the seed via _KVCache._make_tree_task_id(lora_task_id, cache_salt_id) and passing it as the single TreeTaskId argument. The helper returns lora_task_id unchanged when cache_salt_id is None so the unsalted fast path is unaffected; with a salt it returns the SHA-256 digest that binds the two fields together. Signed-off-by: lancelly <108499334+lancelly@users.noreply.github.com>

lancelly · 2026-05-21T02:06:18Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-05-21T02:12:52Z

PR_Github #49540 [ run ] triggered by Bot. Commit: a0824a2 Link to invocation

Build #39108 of PR NVIDIA#14351 failed 7 tests in tests/unittest/disaggregated/ test_router.py with: TypeError: RootBlock.make_key() takes 1 positional argument but 2 were given This PR collapses the prior (lora_task_id, cache_salt_id) pair into a single TreeTaskId seed for the radix tree, but two callers on the base feat/deepseek_v4 branch were not migrated: - tensorrt_llm/serve/router.py — two call sites still pass (None, cache_salt_id) to V2RootBlock.make_key - tests/unittest/disaggregated/test_router.py — two call sites still pass cache_salt_id as the 4th positional argument to sequence_to_blockchain_keys Fix by computing the seed via _KVCache._make_tree_task_id(lora_task_id, cache_salt_id) and passing it as the single TreeTaskId argument. The helper returns lora_task_id unchanged when cache_salt_id is None so the unsalted fast path is unaffected; with a salt it returns the SHA-256 digest that binds the two fields together. Signed-off-by: lancelly <108499334+lancelly@users.noreply.github.com>

lancelly · 2026-05-21T02:18:55Z

/bot run --disable-fail-fast

lancelly · 2026-05-21T02:26:09Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-05-21T02:27:13Z

PR_Github #49544 [ run ] triggered by Bot. Commit: 70263ce Link to invocation

tensorrt-cicd · 2026-05-21T02:31:06Z

PR_Github #49540 [ run ] completed with state ABORTED. Commit: a0824a2

Link to invocation

tensorrt-cicd · 2026-05-21T02:33:58Z

PR_Github #49546 [ run ] triggered by Bot. Commit: 70263ce Link to invocation

After NVIDIA#14140 (cherry-picked in NVIDIA#14353) RootBlock no longer exposes ``lora_task_id`` / ``cache_salt_id`` directly — both fields are folded into a ``ReuseScope`` NamedTuple attached as ``root.reuse_scope``. ``KVCacheEventManager._root_attrs_from_root_block`` was still reading the legacy attribute names via ``getattr(..., None)``, so every emitted event silently received ``(None, None)`` and the V1-compat hash collapsed to the same value for any LoRA/salt request. Downstream Dynamo routing depends on those hashes, so this regression would materially degrade prefix cache hit rate for any request carrying a LoRA task id or a cache salt. Fix ``_root_attrs_from_root_block`` to prefer ``root.reuse_scope`` and fall back to the legacy attributes (keeps any in-flight pre-refactor RootBlock instances working). Also: - Update the now-stale ``test_v2_root_key_distinguishes_lora_from_cache_salt_id`` to use the new single-arg ``RootBlock.make_key(ReuseScope(...))`` API. - Extend ``_FakeRootBlock`` with an optional ``reuse_scope`` kwarg so tests can mimic both the pre- and post-refactor RootBlock shape. - Add a regression test ``test_v2_kv_cache_event_manager_v1_hash_reads_root_reuse_scope`` that asserts (a) a ReuseScope-shaped root and a legacy root with the same lora/salt produce identical V1 event hashes, (b) different scopes still produce different hashes (no silent collapse), and (c) an empty ReuseScope matches a legacy root with no salt/lora. Addresses reviewer feedback on NVIDIA#14351 (peihu-nv). Signed-off-by: lancelly <108499334+lancelly@users.noreply.github.com>

tensorrt-cicd · 2026-05-21T10:33:07Z

PR_Github #49546 [ run ] completed with state SUCCESS. Commit: 70263ce
/LLM/main/L0_MergeRequest_PR pipeline #39176 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

lfr-0531 · 2026-05-21T11:15:32Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-05-21T11:22:37Z

PR_Github #49688 [ run ] triggered by Bot. Commit: 70263ce Link to invocation

tensorrt-cicd · 2026-05-21T13:19:47Z

PR_Github #49688 [ run ] completed with state SUCCESS. Commit: 70263ce
/LLM/main/L0_MergeRequest_PR pipeline #39295 completed with status: 'SUCCESS'

CI Report

Link to invocation

After NVIDIA#14140 (cherry-picked in NVIDIA#14353) RootBlock no longer exposes ``lora_task_id`` / ``cache_salt_id`` directly — both fields are folded into a ``ReuseScope`` NamedTuple attached as ``root.reuse_scope``. ``KVCacheEventManager._root_attrs_from_root_block`` was still reading the legacy attribute names via ``getattr(..., None)``, so every emitted event silently received ``(None, None)`` and the V1-compat hash collapsed to the same value for any LoRA/salt request. Downstream Dynamo routing depends on those hashes, so this regression would materially degrade prefix cache hit rate for any request carrying a LoRA task id or a cache salt. Fix ``_root_attrs_from_root_block`` to prefer ``root.reuse_scope`` and fall back to the legacy attributes (keeps any in-flight pre-refactor RootBlock instances working). Also: - Update the now-stale ``test_v2_root_key_distinguishes_lora_from_cache_salt_id`` to use the new single-arg ``RootBlock.make_key(ReuseScope(...))`` API. - Extend ``_FakeRootBlock`` with an optional ``reuse_scope`` kwarg so tests can mimic both the pre- and post-refactor RootBlock shape. - Add a regression test ``test_v2_kv_cache_event_manager_v1_hash_reads_root_reuse_scope`` that asserts (a) a ReuseScope-shaped root and a legacy root with the same lora/salt produce identical V1 event hashes, (b) different scopes still produce different hashes (no silent collapse), and (c) an empty ReuseScope matches a legacy root with no salt/lora. Addresses reviewer feedback on NVIDIA#14351 (peihu-nv). Signed-off-by: lancelly <108499334+lancelly@users.noreply.github.com>

…VIDIA#13793) (NVIDIA#14351) Signed-off-by: Yueh-Ting Chen <yueh.ting.chen@gmail.com> Signed-off-by: lancelly <108499334+lancelly@users.noreply.github.com> Co-authored-by: Yueh-Ting (eop) Chen <yuehtingc@nvidia.com>

github-actions Bot assigned lancelly May 20, 2026

lancelly changed the title ~~[None][chore] DSv4: replace cache_salt_id impl with main's TreeTaskId (cp #13793)~~ [None][chore] Replace cache_salt_id impl with main's TreeTaskId (cp #13793) May 20, 2026

lancelly mentioned this pull request May 20, 2026

[None][chore] ReuseScope refactor (cp #14140) #14353

Merged

[None][feat] Support cache_salt_id in KV cache v2 manager (NVIDIA#13793)

e457809

Signed-off-by: Yueh-Ting Chen <yueh.ting.chen@gmail.com> (cherry picked from commit 4baeec3) Signed-off-by: lancelly <108499334+lancelly@users.noreply.github.com>

lancelly force-pushed the feat/dsv4-align-kvcm-with-main branch from 0d9c7b9 to e457809 Compare May 20, 2026 10:23

lancelly marked this pull request as ready for review May 20, 2026 15:14

lancelly requested review from a team as code owners May 20, 2026 15:14

lancelly requested review from joyang-nv, laikhtewari and nv-guomingz and removed request for a team May 20, 2026 15:14

peihu-nv approved these changes May 20, 2026

View reviewed changes

lancelly requested a review from a team as a code owner May 21, 2026 02:05

lancelly force-pushed the feat/dsv4-align-kvcm-with-main branch from a0824a2 to eaddc9a Compare May 21, 2026 02:17

Merge branch 'feat/deepseek_v4' into feat/dsv4-align-kvcm-with-main

70263ce

lfr-0531 added the deepseek-v4 label May 21, 2026

pcastonguay merged commit e696714 into NVIDIA:feat/deepseek_v4 May 21, 2026
6 checks passed

lancelly mentioned this pull request May 21, 2026

[None][fix] adp_router: salt-aware probe_prefix_match_length on v1/v2 KV cache backends #14417

Closed

2 tasks

lancelly mentioned this pull request May 22, 2026

[None][feat] adp_router: salt-aware probe_prefix_match_length on v1/v2 KV cache backends #14435

Merged

Uh oh!

Conversation

lancelly commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Conflict resolution notes

Uh oh!

lancelly commented May 20, 2026

Uh oh!

lancelly commented May 20, 2026

Uh oh!

peihu-nv left a comment

Choose a reason for hiding this comment

Uh oh!

peihu-nv commented May 20, 2026

Uh oh!

tensorrt-cicd commented May 20, 2026

Uh oh!

tensorrt-cicd commented May 20, 2026

Uh oh!

lancelly commented May 21, 2026

Uh oh!

tensorrt-cicd commented May 21, 2026

Uh oh!

lancelly commented May 21, 2026

Uh oh!

lancelly commented May 21, 2026

Uh oh!

tensorrt-cicd commented May 21, 2026

Uh oh!

tensorrt-cicd commented May 21, 2026

Uh oh!

tensorrt-cicd commented May 21, 2026

Uh oh!

tensorrt-cicd commented May 21, 2026

Uh oh!

lfr-0531 commented May 21, 2026

Uh oh!

tensorrt-cicd commented May 21, 2026

Uh oh!

tensorrt-cicd commented May 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lancelly commented May 20, 2026 •

edited

Loading