Skip to content

[Stability fix] turn off HMA allocator when connector is set #27592

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 28, 2025
Merged

[Stability fix] turn off HMA allocator when connector is set #27592

merged 6 commits into from
Oct 28, 2025

Conversation

@KuntaiDu
Copy link
Collaborator

@KuntaiDu KuntaiDu commented Oct 27, 2025 •
edited by github-actions bot
Loading

Due to #25712 , currently vLLM will hard fail if the user simply set a connector because vLM enables HMA by default.

To avoid hard fail, before discussing with @njhill to figure out a better solution, let's just turn off HMA when a connector is being set.

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@mergify
Copy link

mergify bot commented Oct 27, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @KuntaiDu.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Oct 27, 2025
gemini-code-assist[bot]
gemini-code-assist bot reviewed Oct 27, 2025
View reviewed changes
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 aims to fix a stability issue by disabling the Hybrid Memory Allocator (HMA) when a connector is configured. While the change is functionally correct in its intent, it introduces a redundant code block. The check for kv_transfer_config is now performed twice consecutively. This should be consolidated into a single check to improve code clarity and maintainability.

@mergify mergify bot removed the needs-rebase label Oct 27, 2025
@KuntaiDu
Copy link
Collaborator Author

KuntaiDu commented Oct 27, 2025

Code Review

This pull request aims to fix a stability issue by disabling the Hybrid Memory Allocator (HMA) when a connector is configured. While the change is functionally correct in its intent, it introduces a redundant code block. The check for kv_transfer_config is now performed twice consecutively. This should be consolidated into a single check to improve code clarity and maintainability.

It's now fixed. Thanks for the reminder.

NickLucche
NickLucche approved these changes Oct 27, 2025
View reviewed changes
Copy link
Collaborator

@NickLucche NickLucche left a comment

Choose a reason for hiding this comment

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

Fixes

(EngineCore_DP0 pid=3595930) ValueError: Connector NixlConnector does not support HMA but HMA is enabled. Please set `--disable-hybrid-kv-cache-manager`.

I think we should just change logic to disabling it if it does not support hma with the interface you defined, given it's on by default

njhill
njhill reviewed Oct 27, 2025
View reviewed changes
Copy link
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

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

Thanks @KuntaiDu

@njhill njhill added this to the v0.11.1 milestone Oct 27, 2025
@KuntaiDu
add logger warning
bb82eb6 
Signed-off-by: KuntaiDu <[email protected]>
@KuntaiDu KuntaiDu requested a review from njhill October 27, 2025 18:21
@KuntaiDu
Copy link
Collaborator Author

KuntaiDu commented Oct 27, 2025

Fixes

(EngineCore_DP0 pid=3595930) ValueError: Connector NixlConnector does not support HMA but HMA is enabled. Please set `--disable-hybrid-kv-cache-manager`.

I think we should just change logic to disabling it if it does not support hma with the interface you defined, given it's on by default

Agree. For now let's temporarily disable it for all connectors to avoid blocking the release. Will figure out a way to only turn off HMA when connector does not support it.

njhill
njhill approved these changes Oct 27, 2025
View reviewed changes
Copy link
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

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

Thanks @KuntaiDu

@njhill njhill added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 27, 2025
@njhill njhill enabled auto-merge (squash) October 27, 2025 18:47
ApostaC
ApostaC approved these changes Oct 27, 2025
View reviewed changes
Copy link
Collaborator

@ApostaC ApostaC left a comment

Choose a reason for hiding this comment

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

LGTM!

@simon-mo
Copy link
Collaborator

simon-mo commented Oct 28, 2025

Ready to be force merged?

@simon-mo simon-mo disabled auto-merge October 28, 2025 01:32
@simon-mo simon-mo merged commit 255e34c into vllm-project:main Oct 28, 2025
42 of 45 checks passed
@KuntaiDu
Copy link
Collaborator Author

KuntaiDu commented Oct 29, 2025

Ready to be force merged?

Yes and ty for the force merge.

@KuntaiDu KuntaiDu deleted the kuntai-disable-HMA-for-connector-for-now branch October 29, 2025 04:31
@KuntaiDu KuntaiDu mentioned this pull request Nov 4, 2025
Merged
5 tasks
ilmarkov pushed a commit to neuralmagic/vllm that referenced this pull request Nov 7, 2025
@KuntaiDu @ilmarkov
[Stability fix] turn off HMA allocator when connector is set (vllm-pr…
992bfa3 
…oject#27592)

Signed-off-by: KuntaiDu <[email protected]>
Signed-off-by: Kuntai Du <[email protected]>
ZhengHongming888 pushed a commit to ZhengHongming888/vllm that referenced this pull request Nov 8, 2025
@KuntaiDu @ZhengHongming888
[Stability fix] turn off HMA allocator when connector is set (vllm-pr…
d52d89e 
…oject#27592)

Signed-off-by: KuntaiDu <[email protected]>
Signed-off-by: Kuntai Du <[email protected]>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
@KuntaiDu @rtourgeman
[Stability fix] turn off HMA allocator when connector is set (vllm-pr…
12eb1be 
…oject#27592)

Signed-off-by: KuntaiDu <[email protected]>
Signed-off-by: Kuntai Du <[email protected]>
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
@KuntaiDu @devpatelio
[Stability fix] turn off HMA allocator when connector is set (vllm-pr…
d4c4db1 
…oject#27592)

Signed-off-by: KuntaiDu <[email protected]>
Signed-off-by: Kuntai Du <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NickLucche NickLucche NickLucche approved these changes

@njhill njhill njhill approved these changes

@ApostaC ApostaC ApostaC approved these changes

@simon-mo simon-mo Awaiting requested review from simon-mo

@WoosukKwon WoosukKwon Awaiting requested review from WoosukKwon WoosukKwon is a code owner

@youkaichao youkaichao Awaiting requested review from youkaichao youkaichao is a code owner

@robertgshaw2-redhat robertgshaw2-redhat Awaiting requested review from robertgshaw2-redhat robertgshaw2-redhat is a code owner

@mgoin mgoin Awaiting requested review from mgoin mgoin is a code owner

@tlrmchlsmth tlrmchlsmth Awaiting requested review from tlrmchlsmth tlrmchlsmth is a code owner

@houseroad houseroad Awaiting requested review from houseroad houseroad is a code owner

@hmellor hmellor Awaiting requested review from hmellor hmellor is a code owner

@yewentao256 yewentao256 Awaiting requested review from yewentao256 yewentao256 is a code owner

@ProExpertProg ProExpertProg Awaiting requested review from ProExpertProg ProExpertProg is a code owner

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Milestone

v0.11.1

Development

Successfully merging this pull request may close these issues.

5 participants

@KuntaiDu @simon-mo @NickLucche @njhill @ApostaC