Skip to content

fix(hicache): isolate decode offload state per request - #37026

Merged
hnyls2002 merged 1 commit into
sgl-project:mainfrom
CyberSecurityErial:fix/decode-offload-request-generation
Aug 30, 2026
Merged

hnyls2002 merged 1 commit into
sgl-project:mainfrom
CyberSecurityErial:fix/decode-offload-request-generation

Conversation

@CyberSecurityErial

@CyberSecurityErial CyberSecurityErial commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Motivation

DecodeKVCacheOffloadManager tracks asynchronous D2H offload state by the caller-provided rid. A completed request is removed from rid_to_state before its copy necessarily finishes, so a new request reusing the same rid can inherit stale progress and be modified by a late callback from the old request.

Reproduction

This reproduces deterministically without relying on GPU timing. It restores only the regression test onto the exact base commit, then runs that single test:

git fetch origin pull/37026/head:pr-37026
git switch --detach 4d53767b09429c67a4137352c762372923853eb6
git restore --source pr-37026 -- test/registered/unit/disaggregation/test_specv2_kvcache_offloading.py
PYTHONPATH=python python3 -m pytest -q \
  test/registered/unit/disaggregation/test_specv2_kvcache_offloading.py \
  -k reused_rid_does_not_share_offload_lifecycle

Expected result on the base commit:

FAILED ...::test_reused_rid_does_not_share_offload_lifecycle
AssertionError: False is not true

The failure is at the second offload_kv_cache(new_req) call: new_req reuses the old request state because both requests have the same rid. The same test passes on this PR.

Modifications

  • Key decode offload state and in-flight counters by the concrete Req instance.
  • Keep rid only for external identity and logging.
  • Add a regression test with two distinct requests that reuse the same rid while the first request's D2H copy is still pending.

Accuracy Tests

Not applicable. This change does not modify the model forward path or model outputs.

The focused regression test passes:

PYTHONPATH=python python3 -m pytest -q test/registered/unit/disaggregation/test_specv2_kvcache_offloading.py

14 passed

Speed Tests and Profiling

Not applicable. This change only fixes asynchronous request bookkeeping and does not modify inference kernels or normal decode computation.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ❌ Run #33257511945
Latest PR Test (Extra): ❌ Run #33257511870
Latest PR Test (AMD ROCm 7.2): ❌ Run #33257511898

Co-authored-by: Guanyi Chen <939416532@qq.com>
@CyberSecurityErial
CyberSecurityErial force-pushed the fix/decode-offload-request-generation branch from 7969778 to 5ae9913 Compare August 29, 2026 14:24
@hnyls2002

Copy link
Copy Markdown
Collaborator

/rerun-test registered/unit/disaggregation/test_specv2_kvcache_offloading.py registered/disaggregation/test_disaggregation_decode_offload.py

@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Results for /rerun-test registered/unit/disaggregation/test_specv2_kvcache_offloading.py registered/disaggregation/test_disaggregation_decode_offload.py:

🚀 ubuntu-latest (1 test): ✅ View workflow run

cd test/ && python3 registered/unit/disaggregation/test_specv2_kvcache_offloading.py

🚀 2-gpu-h100 (1 test): ✅ View workflow run

cd test/ && python3 registered/disaggregation/test_disaggregation_decode_offload.py

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.

2 participants