[Router][Bugfix] Select longest LMCache prefix match - #1055
Conversation
Signed-off-by: 姚远 <asmile.yy@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request updates the KV-cache-aware routing logic to route requests to the instance with the longest matched prefix (maximum matched tokens) instead of simply picking the first instance in the layout info, and adds corresponding unit tests. The review feedback identifies a potential KeyError if the matched instance ID is not found in the active endpoints mapping, suggesting a fallback mechanism to session or QPS routing. Additionally, it recommends stubbing QueryInstMsg in the test suite to prevent potential NameError exceptions when lmcache is not installed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e06a69e64d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: 姚远 <asmile.yy@gmail.com>
|
@ruizhang0101 Thanks for the approval! All checks are green now. Could you help merge this? |
Summary
layout_infoentry with the largestmatched_prefix_lengthLMCache defines
layout_infoasinstance_id -> (location, matched_prefix_length), but does not define dictionary order as a best-match ordering. Its controller builds the mapping while walking chunks, so the first inserted instance is not necessarily the longest match.Tests
The regression test fails twice on current
mainand passes with this change.Static checks run on the changed files:
git diff --checkAll passed.
git commit -s.