Skip to content

Conversation

@tzh21
Copy link
Collaborator

@tzh21 tzh21 commented Oct 13, 2025

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds offline request support and a new method for retrieving static prefill lists. The offline field enables SLO-aware scheduling by allowing lower priority processing for offline requests.

  • Added offline field to request structures for priority-based scheduling
  • Implemented GetStaticPrefillList method throughout the service stack
  • Added protobuf service definition for the new prefill list endpoint

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
xllm_service/scheduler/scheduler.h Added declaration for get_static_prefill_list method
xllm_service/scheduler/scheduler.cpp Implemented get_static_prefill_list method
xllm_service/scheduler/managers/instance_mgr.h Added declaration for get_static_prefill_list method
xllm_service/scheduler/managers/instance_mgr.cpp Implemented get_static_prefill_list method with TODO for refactoring
xllm_service/rpc_service/service.h Added declarations for get_static_prefill_list and GetStaticPrefillList methods
xllm_service/rpc_service/service.cpp Implemented RPC service methods for prefill list retrieval
xllm_service/request/request.h Added offline boolean field to Request struct
xllm_service/proto/xllm_rpc_service.proto Added GetStaticPrefillList RPC service definition
xllm_service/proto/xllm/completion.proto Added optional offline field to CompletionRequest
xllm_service/proto/xllm/chat.proto Added optional offline field to ChatRequest

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +173 to +174
std::vector<std::string> InstanceMgr::get_static_prefill_list(
const std::string& instance_name) {
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The parameter instance_name is not used in the implementation. Consider removing it if not needed, or implement the intended filtering logic if it should be used to filter results.

Copilot uses AI. Check for mistakes.
brpc::ClosureGuard done_guard(done);
std::vector<std::string> prefill_list =
xllm_rpc_service_impl_->get_static_prefill_list(req->name());
for (auto& p : prefill_list) {
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

Using std::move on loop variable p in a range-based for loop can lead to undefined behavior on subsequent iterations. Consider using const auto& for the loop variable or avoid moving.

Suggested change
for (auto& p : prefill_list) {
for (auto p : prefill_list) {

Copilot uses AI. Check for mistakes.
@tzh21 tzh21 changed the title feat: add offline field and GetStaticPrefillList method feat: add offline field and GetStaticPrefillList method. Oct 13, 2025
Copy link
Collaborator

@Kang-Meng Kang-Meng left a comment

Choose a reason for hiding this comment

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

LGTM

@tzh21 tzh21 merged commit 40c17f0 into jd-opensource:main Oct 13, 2025
1 check passed
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.

3 participants