Skip to content

[Bugfix][Router] Seed the prefix trie on the below-threshold QPS fallback path#990

Open
vedcsolution wants to merge 2 commits into
vllm-project:mainfrom
vedcsolution:fix-prefixaware-min-match-seeding
Open

[Bugfix][Router] Seed the prefix trie on the below-threshold QPS fallback path#990
vedcsolution wants to merge 2 commits into
vllm-project:mainfrom
vedcsolution:fix-prefixaware-min-match-seeding

Conversation

@vedcsolution

Copy link
Copy Markdown

Bug

With --prefix-min-match-length set to any value > 0 (added in #959), PrefixAwareRouter never activates prefix affinity:

  1. The router starts with an empty HashTrie.
  2. Every incoming request therefore gets match_length = 0 < prefix_min_match_length.
  3. The below-threshold branch returns self._qps_routing(...) without inserting the prompt into the trie.
  4. The trie stays empty forever, so every subsequent request also takes the fallback branch.

Net effect: setting the flag silently turns the prefix-aware router into a pure QPS router. We hit this in production (two vLLM backends behind the router): with --prefix-min-match-length 4096 the trie never seeded and no request ever pinned; with 0 affinity works but shared prompt prefixes (system prompts / shared project context) create a self-reinforcing single-backend monopoly — the exact problem the threshold was designed to solve.

Fix

On the below-threshold path, still insert the prompt into the trie, attributed to the QPS-selected endpoint:

Verified in production with two vLLM backends and --prefix-min-match-length 100000: identical sub-threshold prompts QPS-balance across both backends, distinct conversations sharing a 40k-char prefix spread across both backends, and a >100k-char conversation pins to a single backend across turns.

Tests

  • Updated test_route_falls_back_to_qps_when_match_below_threshold: the fallback path must insert the prompt attributed to the QPS-selected endpoint (previously asserted no insert, which encoded the bug).
  • Added test_below_threshold_seeding_enables_later_pinning: regression test using the real HashTrie — a cold-trie request routes by QPS and seeds the trie; a follow-up request extending the same prompt matches above the threshold and pins to the seeding endpoint even when QPS stats prefer the other one.

All 4 tests in src/tests/test_prefixaware_router.py pass.

…back path

With prefix_min_match_length > 0, PrefixAwareRouter starts with an empty
HashTrie: every request matches below the threshold, falls back to QPS
routing, and returns without inserting the prompt. The trie never seeds,
so prefix affinity never activates and the router permanently degrades
to pure QPS routing.

Insert the prompt into the trie on the fallback path, attributed to the
QPS-selected endpoint. Short or new prefixes keep load-balancing across
endpoints, and once a request's prefix history exceeds the threshold it
pins to the endpoint that already holds its KV cache, which is the
intended semantics of vllm-project#959.

Signed-off-by: carlosmolina0615 <carlosmolina0615@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request ensures that prompts are inserted into the trie even when falling back to QPS-based routing due to a match length below the threshold, resolving an issue where a cold trie would never seed. A regression test has been added to verify this behavior. The review feedback points out a potential issue where an empty endpoints list could cause the QPS routing to return None, which would then pollute the trie. A guard should be added to prevent inserting None values into the trie.

Comment on lines +513 to +515
selected_endpoint = self._qps_routing(endpoints, request_stats)
await self.hashtrie.insert(prompt, selected_endpoint)
return selected_endpoint

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If endpoints is empty, self._qps_routing returns None. Calling self.hashtrie.insert with None will pollute the trie with invalid endpoints, which can cause future routing requests to incorrectly resolve to None. We should guard against inserting None into the trie.

            selected_endpoint = self._qps_routing(endpoints, request_stats)\n            if selected_endpoint is not None:\n                await self.hashtrie.insert(prompt, selected_endpoint)\n            return selected_endpoint

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch — guard added in b105ae8. _qps_routing indeed returns None on an empty endpoint list, so the fallback path now only inserts when an endpoint was actually selected.

…re available

Signed-off-by: carlosmolina0615 <carlosmolina0615@gmail.com>
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.

1 participant