-
-
Notifications
You must be signed in to change notification settings - Fork 12.2k
[P/D][v1] Allow registering external kv connector from args #18028
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
[P/D][v1] Allow registering external kv connector from args #18028
Conversation
|
👋 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 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 🚀 |
|
cc @njhill for review |
| connector_name = config.kv_transfer_config.kv_connector | ||
| if (config.kv_transfer_config.kv_connector_external_registration_args | ||
| is not None and connector_name not in cls._registry): | ||
| cls.register_connector(**config.kv_transfer_config. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this be called multiple times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, based on current v1 design each process will only call it once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double checked, when running world_size = 1, UniProcExecutor will be used, scheduler & worker will run on the same process and this will be called twice.
Lines 1828 to 1829 in d67085c
| if self.distributed_executor_backend is None and self.world_size == 1: | |
| self.distributed_executor_backend = "uni" |
|
Interesting feature, it will help private kv connector implementation works without change or hack vllm, this feature is very useful. |
|
@liuzijing2014 thanks for the PR—this is definitely a step in the right direction (from the GenericKVConnector #17840) toward supporting external connector integration. I'd like to offer an alternative approach that I believe simplifies things further: #18142. Rather than dynamically registering the external KV connector through an opaque config field (encapsulating multiple fields: connector name, module path, class name), the factory can directly load the external implementation using a single, semantically clear config field: the external module path. This eliminates the need for explicit registration or wrapper classes. More importantly, with this approach:
This minimizes boilerplate and improves usability while preserving full flexibility. The supported external connectors can simply be documented or included in examples, rather than embedded in the core repo. I’d be glad to hear your thoughts. |
|
This pull request has merge conflicts that must be resolved before it can be |
|
This pull request has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this pull request should remain open. Thank you! |
|
This pull request has been automatically closed due to inactivity. Please feel free to reopen if you intend to continue working on it. Thank you! |
KVConnectorBase_V1 API is introduced in (#15960), however, due to the multi-process nature, it's hard to register a customized kv connector implementations from outside context.
This change will make the kv connector registration more flexible as it allows for external dependency injection.
Example Usage:
This will allow external developers to register customized kv connectors without keep adding a boilerplate & updating the factory.py.
The third commit moves the kv connector init after model loading, allowing
vllm_configto be populated with all attention layers info.