Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/grpc_client/proto/sglang_scheduler.proto
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ message GetServerInfoResponse {
// Server metadata
string server_type = 8; // "grpc"
google.protobuf.Timestamp start_time = 9;
int32 max_total_num_tokens = 10;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The addition of max_total_num_tokens to GetServerInfoResponse is a good step towards aligning gRPC server information with the HTTP server. However, please ensure that the corresponding Python servicer's GetServerInfo method (in grpc_servicer/smg_grpc_servicer/sglang/servicer.py) is updated to populate this new field with the correct value. Currently, gRPC clients will receive the default value (0), which might lead to incorrect information being reported.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Populate max_total_num_tokens before advertising it

The schema now exposes max_total_num_tokens, but the only SGLang gRPC implementation I inspected still never sets it: grpc_servicer/smg_grpc_servicer/sglang/servicer.py:491-500 builds GetServerInfoResponse without this field. Because proto3 defaults unset int32 fields to 0, every GetServerInfo reply from the gRPC server will report a bogus token limit until the servicer is updated, which breaks any client that starts using this new field for capacity/context-length decisions.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Regenerate the checked-in Go bindings for this proto change

This API change is not propagated to the repository's Go SDK. bindings/golang/internal/proto/sglang_scheduler.pb.go:2820-2931 still defines GetServerInfoResponse with fields only through start_time, so Go clients built from this commit will silently drop field 10 on decode and have no accessor for it. If this proto stays source-controlled, the generated Go files need to be refreshed in the same change.

Useful? React with 👍 / 👎.


// Note: internal_states not provided in gRPC mode
// Scheduler-side metrics (memory usage, throughput) require
Expand Down
1 change: 1 addition & 0 deletions grpc_servicer/smg_grpc_servicer/sglang/servicer.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ def make_serializable(obj):
sglang_version=sglang.__version__,
server_type="grpc",
start_time=start_timestamp,
max_total_num_tokens=self.scheduler_info.get("max_total_num_tokens", 0),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reuse the existing context-length fallback here

If the scheduler bootstrap dict does not contain max_total_num_tokens—a case the same startup path already anticipates in grpc_servicer/smg_grpc_servicer/sglang/server.py:81-85 by falling back to server_args.context_length or 8192—GetServerInfo will now advertise 0 for the new field. Any client that starts using this field for capacity/context-length decisions will treat that worker as having no usable context, even though model_info.max_context_length from the same process remains non-zero.

Useful? React with 👍 / 👎.

)

async def GetLoads(
Expand Down
Loading