Skip to content

[LMCache][Example] Align the PYTHONHASHSEED for prefillers and decoders for KV chunks hashing #21161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zejunchen-zejun
Copy link

@zejunchen-zejun zejunchen-zejun commented Jul 18, 2025

Add and align the PYTHONHASHSEED for prefillers and decoders for hashing the chunks.
For now the LMCache uses the hash seed from the vllm, so it is needed to align the hash seed for all prefillers and decoders, so that the decoders can identify and retrieve the kv cache chunks saved by the prefillers.
https://github.com/LMCache/LMCache/blob/dev/lmcache/v1/token_database.py#L106-L107

@mergify mergify bot added the documentation Improvements or additions to documentation label Jul 18, 2025
Copy link
Contributor

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

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 aligns the PYTHONHASHSEED for LMCache examples to ensure compatibility between prefillers and decoders. However, it introduces a security vulnerability by hardcoding the seed to a fixed value. My review includes a critical comment to address this by making the seed configurable and adding a security warning, which is crucial even for an example script.

Copy link

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

@zejunchen-zejun zejunchen-zejun changed the title [LMCache][Example] Align the PYTHONHASHSEED for prefillers and decoders for hashing the chunks [LMCache][Example] Align the PYTHONHASHSEED for prefillers and decoders for chunks hashing Jul 18, 2025
@zejunchen-zejun zejunchen-zejun changed the title [LMCache][Example] Align the PYTHONHASHSEED for prefillers and decoders for chunks hashing [LMCache][Example] Align the PYTHONHASHSEED for prefillers and decoders for KV chunks hashing Jul 18, 2025
@zejunchen-zejun
Copy link
Author

Hi, @ApostaC @KuntaiDu

For the issue LMCache/LMCache#1095, we add the flag to align the hash seed in the lmcache example code in vllm, could you help review?

Thank you. :)

@ApostaC
Copy link
Collaborator

ApostaC commented Jul 23, 2025

Hi @zejunchen-zejun , this looks good and thanks for the contribution! Can you fix the DCO issue by sign off the commits? Please follow the instructions here: https://docs.vllm.ai/en/latest/contributing/#dco-and-signed-off-by

Also, you may want to setup and run pre-commit locally to lint the code. It can also help you sign off the commits automatically.

@zejunchen-zejun zejunchen-zejun force-pushed the zejun/revise_lmcache_example branch from 9189093 to 1bf6513 Compare July 24, 2025 07:09
and decoders for KV chunks hashing

Signed-off-by: zejunchen-zejun <zejun.chen@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants