Skip to content

refactor: refactor instance_mgr, split components.#51

Open
magicheng0816 wants to merge 1 commit intojd-opensource:mainfrom
magicheng0816:refactor_instance_manager
Open

refactor: refactor instance_mgr, split components.#51
magicheng0816 wants to merge 1 commit intojd-opensource:mainfrom
magicheng0816:refactor_instance_manager

Conversation

@magicheng0816
Copy link
Copy Markdown
Collaborator

Currently, the responsibilities of the instance manager are too heavy, with increasing logic being added, and there is no clear boundary between various parts of the logic. Therefore, the instance manager needs to be split up:

  1. instance_topology, responsible for instance registration and status maintenance; 2. instance_metrics, responsible for collecting and reporting instance metrics; 3. scheduling strategy, which is gradually being migrated to loadbalance_policy, but not completely yet, and requires further refactoring in the future.

Copy link
Copy Markdown

@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 refactors the InstanceMgr by decomposing its responsibilities into three specialized components: InstanceMetrics, InstanceTopology, and InstanceKVCache. This modularization improves the management of cluster state, metrics tracking, and KV cache locations. As part of this change, GlobalKVCacheMgr has been renamed and integrated into InstanceMgr, and load balancing policies have been updated to use the refactored interfaces. A critical regression was identified in the shared round-robin routing logic, which now strictly requires a decode instance to be present for scheduling to succeed; a suggestion has been provided to restore the previous behavior and allow scheduling to proceed with only prefill instances.

Comment thread xllm_service/scheduler/loadbalance_policy/round_robin.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant