Auto-enable router dp_aware routing when DP attention is on - #1351
Auto-enable router dp_aware routing when DP attention is on#1351Shi-Dong wants to merge 2 commits into
Conversation
When sglang_dp_size > 1, DP attention shards the KV cache per DP rank, so each rank keeps its own radix prefix tree. Without DP-aware routing the sgl-router dispatches per-engine and SGLang scatters requests across ranks by load, fragmenting the prefix cache and sharply lowering the KV-cache hit rate. Auto-enable router_args.dp_aware in the sgl-router path when dp_size > 1, honoring an explicit --router-dp-aware and a MILES_DISABLE_AUTO_DP_AWARE=1 opt-out (which warns). Scoped to the sgl-router branch; the miles-router and external-router paths are untouched.
There was a problem hiding this comment.
Code Review
This pull request introduces automatic DP-aware routing when DP attention is enabled (sglang_dp_size > 1) to preserve prefix-cache locality, along with an opt-out mechanism via the MILES_DISABLE_AUTO_DP_AWARE environment variable. It also adds unit tests to verify this behavior. Feedback was provided regarding a potential TypeError if sglang_dp_size is explicitly set to None, and an AttributeError if router_args lacks the dp_aware attribute, suggesting safer attribute retrieval.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if getattr(args, "sglang_dp_size", 1) <= 1 or router_args.dp_aware: | ||
| return |
There was a problem hiding this comment.
If args.sglang_dp_size is explicitly set to None (which is common for optional CLI arguments), getattr(args, "sglang_dp_size", 1) will return None instead of the default 1. This will result in a TypeError: '<=' not supported between instances of 'NoneType' and 'int' when evaluating None <= 1.
Additionally, to be more defensive and avoid potential AttributeErrors if router_args does not have the dp_aware attribute, we should use getattr(router_args, "dp_aware", False).
dp_size = getattr(args, "sglang_dp_size", 1) or 1
dp_aware = getattr(router_args, "dp_aware", False)
if dp_size <= 1 or dp_aware:
returnAddress gemini-code-assist review: getattr(args, "sglang_dp_size", 1) returns None (not 1) when the attribute is present but None, raising TypeError on None <= 1. Coalesce with "or 1", and read router_args.dp_aware defensively via getattr. Add a dp_size=None test case.
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def _maybe_enable_router_dp_aware(args, router_args) -> None: |
There was a problem hiding this comment.
I suggest put this implicit argument setting in arguments.py
|
This may break PD disaggregation if DP aware does not work correctly with PD disaggregation yet. |
|
DP aware support for PD router is in: smg-project/smg#1522 This needs to be added to https://github.com/radixark/sgl-router-for-miles I think. |
Summary
When DP attention is enabled (
--sglang-data-parallel-size > 1, which Miles already forces alongside--sglang-enable-dp-attention), the SGLang engine shards its KV cache across DP ranks — each rank keeps an independent KV pool and its own radix prefix tree. The sgl-router, however, defaults todp_aware = False, so it dispatches at engine granularity and lets SGLang scatter requests across DP ranks by load. That fragments the radix prefix cache across ranks and can sharply reduce the KV-cache hit rate (a shared prefix that used to be cached once is recomputed on every rank it lands on).This PR couples the two: in the sgl-router path, when
sglang_dp_size > 1, Miles now auto-enablesrouter_args.dp_awareso the router routes at DP-rank granularity (preserving prefix-cache locality).Behavior
start_router's non-use_miles_routerbranch). The miles-router path (RadixTreeMiddleware) and externally-managed routers (--sglang-router-ipset) are untouched.--router-dp-aware(alreadyTrue→ left as-is).MILES_DISABLE_AUTO_DP_AWARE=1. Because--router-dp-awareis astore_trueflag (no native "explicit False"), the env var is the supported way to keep DP attention on while routing per-engine — and it warns when used, since it reintroduces the prefix-cache fragmentation.cache_aware;dp_awareis orthogonal, controlling worker granularity rather than the selection policy).Test plan
tests/fast/router/test_router_dp_aware.py) covering the four branches: auto-enable whendp_size > 1, no-op whendp_size == 1, respect an explicitdp_aware=True, and opt-out keeps it disabled + warns.dp_size >= 2) confirming the router log showsdp_awareactive and the KV-cache prefix hit-rate metric recovers vs. a baseline without the change.