[bugfix] Update server type checks for merged PD instances #65
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JohnLiu97Huawei
force-pushed
the
bugfix-1
branch
from
f5536d7 to
2ecfc76
Compare
lm_service/apis/vllm/proxy.py
Outdated
|
|
||
| for server_type in SERVER_PARAMS_MAP: | ||
| if (self.is_pd_merged and server_type == ServerType.P_INSTANCE) or ( | ||
| if ( |
Collaborator
There was a problem hiding this comment.
能不能写个active_map，或者优化一下if判断
JohnLiu97Huawei
force-pushed
the
bugfix-1
branch
from
2ecfc76 to
18a31a1
Compare
| self.is_pd_merged = self.metastore_client.is_pd_merged | ||
| init_params = locals() | ||
| for server_type in SERVER_PARAMS_MAP: | ||
| if (self.is_pd_merged and server_type == ServerType.P_INSTANCE) or ( |
JohnLiu97Huawei
force-pushed
the
bugfix-1
branch
from
18a31a1 to
8c8bd0e
Compare
lm_service/apis/vllm/proxy.py
Outdated
| not self.is_pd_merged and server_type == ServerType.PD_INSTANCE | ||
| ): | ||
| continue | ||
| active_server_types = [ServerType.E_INSTANCE] + ( |
Collaborator
There was a problem hiding this comment.
能不能提取个公共的参数？self.active_server_types避免多次遍历
Expanded the condition to skip both P_INSTANCE and D_INSTANCE when is_pd_merged is True, ensuring correct server type handling in the Proxy class. Signed-off-by: John Liu BUAA <[email protected]>
JohnLiu97Huawei
force-pushed
the
bugfix-1
branch
from
8c8bd0e to
f29aa97
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request refactors the logic for initializing server clusters in
lm_service/apis/vllm/proxy.pyto improve clarity and correctness when handling different server types based on theis_pd_mergedflag. The changes streamline which server types are initialized and adjust the conditions for skipping certain types.Cluster initialization logic improvements:
_init_cluster_with_addr_listto explicitly construct the list of active server types based on theis_pd_mergedflag, replacing the previous loop and conditional logic for skipping server types._init_cluster_with_metastoreto skip bothP_INSTANCEandD_INSTANCEwhenis_pd_mergedis true, ensuring only relevant server types are initialized.