Skip to content

[PD] Use one logical-range contract for DSA state indices - #37770

Closed
IvanShan177 wants to merge 1 commit into
sgl-project:mainfrom
IvanShan177:fix/pd-dsa-state-contract
Closed

IvanShan177 wants to merge 1 commit into
sgl-project:mainfrom
IvanShan177:fix/pd-dsa-state-contract

Conversation

@IvanShan177

@IvanShan177 IvanShan177 commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

This is the first implementation slice of #37765 and contributes to the shared-protocol work tracked by #34510 and #33861.

DSA indexer state is request-scoped. Decode already builds its destination indices from the full logical input length and the device KV pool page size. Prefill currently reuses the generic full-KV helper, which is based on:

seq_len = min(req.extend_range.end, len(req.origin_input_ids))

and the allocator page size. This couples DSA state shape to the locally materialized Prefill range and to an unqualified page-size domain. A cache-hit or chunked-Prefill transition can therefore produce a source state count different from the destination registered by Decode.

Changes

  • Add get_dsa_state_page_indices() as the shared request-scoped DSA index builder.
  • Make both Prefill and Decode use the logical input length and device KV pool page size through that helper.
  • Reject an invalid page size or a logical range larger than the request-to-token row before transport submission.
  • Add a CPU regression test where:
    • logical input length is 129 tokens;
    • Prefill extend_end is 64;
    • allocator page size is 1;
    • device DSA page size is 64.

The expected DSA payload contains three device pages. It is independent of the shorter Prefill extend range and the unrelated allocator page size.

Scope

This PR intentionally does not change the wire format or transport state machine. It also does not implement repacking between different Prefill and Decode device page sizes; that work remains tracked in #37765. The explicit request-role truth table is already covered by #34977 and is not duplicated here.

Validation

  • Linux CPU: the new regression file plus the existing PD wire and NIXL unit tests passed: 75 passed, 12 subtests passed.
  • New regression file alone: 3 passed.
  • Static checks passed for all changed files: ruff format, the repository ruff rule set, isort, registered-test validation, Python compilation, and git diff --check.
  • An equal-page-size NIXL Prefill/Decode GPU run produced matching DSA counts for both baseline and this candidate, including a full cache hit. It did not enter the range-divergence state, so this is no-regression evidence rather than a current-main reproduction.

Follow-ups

  • Carry named source/destination page-layout metadata in the common transfer plan.
  • Add an explicit repack path for heterogeneous device page sizes instead of clipping state-index arrays.
  • Propagate plan-validation errors as terminal per-room failures.

CI States

Latest PR Test (Base): ❌ Run #33740502212
Latest PR Test (Extra): ❌ Run #33740501887
Latest PR Test (AMD ROCm 7.2): ❌ Run #33740501912

@IvanShan177
IvanShan177 force-pushed the fix/pd-dsa-state-contract branch from 04f3398 to 52acb82 Compare September 3, 2026 09:44

@ShangmingCai ShangmingCai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need rebase.

Is this PR ready for review? If so, please convert it from draft to a ready PR.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks @IvanShan177. Closing this because it is still a draft and has not been updated in 30 days.

Reopen it if the work is still relevant.

Some directories moved recently, so an older branch may need retargeting:
sgl-kernel/ -> python/sglang/kernels/aot/, python/sglang/jit_kernel/
-> python/sglang/kernels/jit/, docs/ -> docs/docs/ (.mdx),
bench_serving.py -> benchmark/serving.py, test/srt/ -> test/registered/.

@github-actions github-actions Bot closed this Oct 12, 2026
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