Skip to content

[None][chore] Remove the disagg transfer admission controller - #17245

Closed
Tabrizian wants to merge 1 commit into
NVIDIA:mainfrom
Tabrizian:user/itabrizian/remove-disagg-transfer-admission-controller
Closed

[None][chore] Remove the disagg transfer admission controller#17245
Tabrizian wants to merge 1 commit into
NVIDIA:mainfrom
Tabrizian:user/itabrizian/remove-disagg-transfer-admission-controller

Conversation

@Tabrizian

@Tabrizian Tabrizian commented Aug 4, 2026

Copy link
Copy Markdown
Member

Description

Removes the FCFS admission gate for disaggregated generation KV transfers from
the PyTorch executor (DisaggTransferAdmissionController), introduced in
b6eacd1 ([TRTLLM-12721][fix] Bound V2 context transfer polling, #15356).

Removed along with it, because each became dead once the gate was gone:

  • DisaggTransferAdmissionController / DisaggTransferAdmissionResult
  • _get_disagg_transfer_admission_controller, _apply_disagg_transfer_admission
  • _revert_deferred_disagg_gen_init_alloc, _uses_kv_manager_v2 — only ever
    called to undo KV growth for gate-deferred requests
  • _revert_ctx_alloc — no remaining callers
  • The wait_for_disagg_gen_transfer_progress flag threaded through
    SerializableSchedulerOutput and the PP schedule broadcast. With the gate
    gone it is permanently False, which made the generation-side branch of
    _check_disagg_transfer_progress_when_idle (and its
    _sync_disagg_gen_status_entry allreduce) unreachable, so both are removed.

One behavioral note beyond dead-code deletion: in the PP loop, non-rank-0 ranks
still run scheduler.schedule_request(...) for its request-state side effects,
but no longer reconcile their local disagg candidates against rank 0's admitted
set. That reconciliation only had an effect when the gate deferred requests, so
it is a no-op now.

Deliberately left untouched:

  • The C++ DisaggTransferAdmissionController
    (cpp/include/tensorrt_llm/batch_manager/disaggTransferAdmissionController.h
    and its trtGptModelInflightBatching wiring) used by the TensorRT backend.
  • The max_tokens_in_buffer / kv_transfer_poll_interval_ms fields on
    CacheTransceiverConfig, so this PR carries no public API change.

Test Coverage

This PR only removes code, so coverage is the existing suites minus the tests
for the removed paths:

  • tests/unittest/_torch/executor/test_py_executor.py — removed
    TestDisaggTransferAdmissionController (8 tests) and
    TestDisaggTransferAdmissionPP (2 tests). In TestDisaggTransferIdleProgress,
    the two tests covering the generation-side branch are removed; the two
    context-side tests are kept and updated for the new signature, so the
    remaining idle-progress behavior stays covered.
  • tests/unittest/_torch/executor/test_scheduler_serializable_output.py
    SerializableSchedulerOutput pickle round-trip still covered, minus the
    removed flag.
  • Disaggregated serving integration tests
    (tests/integration/defs/accuracy/test_disaggregated_serving.py) exercise the
    end-to-end path this code sat on.

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

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

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

Summary by CodeRabbit

  • Bug Fixes

    • Simplified scheduling and transfer-progress handling for more consistent execution.
    • Removed obsolete transfer-admission behavior that could defer or roll back scheduling allocations.
    • Improved idle transfer-progress synchronization for context-side scheduling.
  • Refactor

    • Simplified scheduler output serialization by removing an unused transfer-progress status field.
  • Tests

    • Updated executor and scheduler coverage to reflect the streamlined scheduling behavior.

Remove the FCFS admission gate for disaggregated generation KV transfers
from the PyTorch executor, along with the plumbing that existed only to
serve it:

- `DisaggTransferAdmissionController` / `DisaggTransferAdmissionResult`
- `_get_disagg_transfer_admission_controller`, `_apply_disagg_transfer_admission`
- `_revert_deferred_disagg_gen_init_alloc`, `_uses_kv_manager_v2`, `_revert_ctx_alloc`
- The `wait_for_disagg_gen_transfer_progress` flag threaded through
  `SerializableSchedulerOutput` and the PP schedule broadcast, and the
  generation-side branch of `_check_disagg_transfer_progress_when_idle`
  it gated (`_sync_disagg_gen_status_entry`)

The `max_tokens_in_buffer` / `kv_transfer_poll_interval_ms` cache
transceiver config fields and the C++ `DisaggTransferAdmissionController`
used by the TensorRT backend are left untouched.

Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
@Tabrizian

Copy link
Copy Markdown
Member Author

/bot run --disable-fail-fast

@Tabrizian Tabrizian closed this Aug 4, 2026
@Tabrizian
Tabrizian deleted the user/itabrizian/remove-disagg-transfer-admission-controller branch August 4, 2026 07:26
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 208ddff4-1abb-407e-8b67-de301f148568

📥 Commits

Reviewing files that changed from the base of the PR and between 6af4e0b and 2e982e4.

📒 Files selected for processing (4)
  • tensorrt_llm/_torch/pyexecutor/py_executor.py
  • tensorrt_llm/_torch/pyexecutor/scheduler/scheduler.py
  • tests/unittest/_torch/executor/test_py_executor.py
  • tests/unittest/_torch/executor/test_scheduler_serializable_output.py

📝 Walkthrough

Walkthrough

Changes

Disaggregated transfer admission removal

Layer / File(s) Summary
Simplify pipeline scheduling
tensorrt_llm/_torch/pyexecutor/py_executor.py, tests/unittest/_torch/executor/test_py_executor.py
Pipeline-parallel scheduling no longer applies generation-transfer admission, propagates its wait flag, or rolls back deferred allocations. Related tests and imports were removed.
Retain context-side idle transfer checks
tensorrt_llm/_torch/pyexecutor/py_executor.py, tests/unittest/_torch/executor/test_py_executor.py
Idle transfer handling now checks context-side progress only. The tests cover context fallback and peer-rank polling.
Update serialized scheduler output
tensorrt_llm/_torch/pyexecutor/scheduler/scheduler.py, tests/unittest/_torch/executor/test_scheduler_serializable_output.py
SerializableSchedulerOutput no longer stores or reconstructs wait_for_disagg_gen_transfer_progress.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: juney-nvidia

✨ Finishing Touches 💡 2
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch user/itabrizian/remove-disagg-transfer-admission-controller
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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.

1 participant