[Bugfix] Forward session headers to consistent-hash routing in PD discovery mode - #243
Conversation
Signed-off-by: simondanielsson <simon.danielsson99@hotmail.com>
Signed-off-by: simondanielsson <simon.danielsson99@hotmail.com>
Signed-off-by: simondanielsson <simon.danielsson99@hotmail.com>
|
fix rust checks please |
Signed-off-by: simondanielsson <simon.danielsson99@hotmail.com>
…-discovery-consistent-hash Signed-off-by: simondanielsson <simon.danielsson99@hotmail.com>
|
Environment issue in NV PD tests causing CI to fail: https://buildkite.com/vllm/router/builds/786/canvas?sid=01a0812d-1087-4bb6-92ac-2546f27be662&tab=output |
|
@hsliuustc0106 Who can I contact to get a review on this PR? Thanks |
There was a problem hiding this comment.
🟡 Changes recommended
The new per-request header normalization introduces avoidable allocations and should be gated on whether the active policies actually need headers, using ASCII-only lowercasing for header names.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes session affinity in vLLM prefill/decode (PD) discovery mode by ensuring request headers (notably X-Session-Id) are forwarded into policy-based worker selection, so consistent-hash routing can remain stable across requests.
Changes:
- Normalize incoming HTTP headers and pass them into policy-based worker selection for PD discovery routing.
- Update discovery-mode worker selection to use the header-aware policy selection API.
- Add tests verifying consistent-hash routing uses session headers (and that cache-aware routing ignores them).
File summaries
| File | Description |
|---|---|
| src/routers/http/vllm_pd_router.rs | Normalizes headers for discovery-mode selection, threads headers into policy-based routing, and adds regression tests for session-affinity behavior. |
Review details
Suppressed comments (1)
src/routers/http/vllm_pd_router.rs:608
process_vllm_requestnormalizes all headers into a new HashMap for every request, even when the active policies don’t consult headers. This adds avoidable per-request allocations; you can gate the normalization onneeds_headers()for the prefill/decode policies and passNoneotherwise.
let request_text = serde_json::to_string(&request_json).ok();
let request_str = request_text.as_deref();
let request_headers = headers.map(normalize_request_headers);
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: simondanielsson <simon.danielsson99@hotmail.com>
Purpose
This preserves session affinity for both completions and chat completions when running in
--vllm-pd-disaggrationmode with service discovery.Previously, the X-Session-Id header was ignored in that case.
Test Plan
CI passes.
Test Result
OK
Essential Elements of an Effective PR Description Checklist