Split DP cache affinity from load balancing - #26186
andrewdoro wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a cache affinity strategy for data parallel dispatch, enabling requests with the same routing key to be consistently routed to the same worker rank via a new --dp-cache-affinity argument. The review identifies a potential memory leak due to the unbounded routing_key_to_dp_rank dictionary and suggests implementing a size limit or clearing mechanism to manage memory usage.
| @@ -154,6 +176,7 @@ def __init__( | |||
|
|
|||
| # Load balance budget | |||
| self.dp_budget = DPBudget(server_args.dp_size) | |||
| self.routing_key_to_dp_rank: dict[str, int] = {} | |||
There was a problem hiding this comment.
The routing_key_to_dp_rank dictionary is currently unbounded. In a long-running server environment, if clients provide many unique routing keys (e.g., unique session IDs or request IDs), this dictionary will grow indefinitely, leading to a memory leak. Consider using a bounded cache (like an LRU cache) or implementing a size limit to prevent excessive memory consumption.
| def remember_cache_affinity_rank(self, req: Req, target_rank: int): | ||
| if self.dp_cache_affinity_method != DPCacheAffinityMethod.ROUTING_KEY: | ||
| return | ||
| if req.routing_key: | ||
| self.routing_key_to_dp_rank[req.routing_key] = target_rank |
There was a problem hiding this comment.
To prevent the routing_key_to_dp_rank dictionary from growing indefinitely, it is recommended to enforce a maximum size. A simple approach is to clear the dictionary or remove the oldest entries when a certain threshold is reached.
| def remember_cache_affinity_rank(self, req: Req, target_rank: int): | |
| if self.dp_cache_affinity_method != DPCacheAffinityMethod.ROUTING_KEY: | |
| return | |
| if req.routing_key: | |
| self.routing_key_to_dp_rank[req.routing_key] = target_rank | |
| def remember_cache_affinity_rank(self, req: Req, target_rank: int): | |
| if self.dp_cache_affinity_method != DPCacheAffinityMethod.ROUTING_KEY: | |
| return | |
| if req.routing_key: | |
| if len(self.routing_key_to_dp_rank) >= 10000: | |
| # Simple heuristic to prevent unbounded growth | |
| self.routing_key_to_dp_rank.clear() | |
| self.routing_key_to_dp_rank[req.routing_key] = target_rank |
c800402 to
bbf2ef9
Compare
bbf2ef9 to
68a7ccb
Compare
|
Thanks @andrewdoro. Closing this because it has had no updates in 105 days. Reopen it if the work is still relevant. Some directories moved recently, so an older branch may need retargeting: |
Summary
--dp-cache-affinity routing_keyas an optional DP dispatch override layered on top of--load-balance-methodrouted_dp_rankrouting highest priorityfollow_bootstrap_roomFixes #26066
Tests
python -m py_compile python/sglang/srt/managers/data_parallel_controller.py python/sglang/srt/server_args.py python/sglang/srt/disaggregation/common/conn.py test/registered/unit/managers/test_dp_budget.py test/registered/unit/server_args/test_server_args.py test/registered/unit/disaggregation/test_register_to_bootstrap.pyCUDA_VISIBLE_DEVICES=9 PYTHONPATH=python pytest test/registered/unit/managers/test_dp_budget.py test/registered/unit/server_args/test_server_args.py test/registered/unit/disaggregation/test_register_to_bootstrap.py -q-> 68 passed, 4 subtests passedCI States
Latest PR Test (Base): ❌ Run #26346277063
Latest PR Test (Extra): ❌ Run #26346277029