Skip to content
Closed
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
12 changes: 9 additions & 3 deletions python/sglang/srt/managers/data_parallel_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,10 @@ def launch_tensor_parallel_group(

self.max_total_num_tokens = scheduler_info[0]["max_total_num_tokens"]
self.max_req_input_len = scheduler_info[0]["max_req_input_len"]
# Retain the full first child init dict so callers that build
# the upstream handshake (run_data_parallel_controller_process,
# below) can forward every field — not just the two limits.
self.scheduler_init_info = scheduler_info[0]

def maybe_external_dp_rank_routing(self, req: Req):
if req.routed_dp_rank is not None:
Expand Down Expand Up @@ -649,10 +653,12 @@ def run_data_parallel_controller_process(
proc.pid for proc in controller.scheduler_procs if proc is not None
]
pipe_writer.send(
# Spread the first child scheduler's full init dict (status +
# both max_* limits + the multimodal/tokenizer metadata added
# by the get_init_info patch above) so dp_size > 1 doesn't
# strip them before they reach the gRPC servicer.
{
"status": "ready",
"max_total_num_tokens": controller.max_total_num_tokens,
"max_req_input_len": controller.max_req_input_len,
**controller.scheduler_init_info,
SCHEDULER_PIDS_ARG: scheduler_pids,
}
)
Expand Down
22 changes: 22 additions & 0 deletions python/sglang/srt/managers/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1494,10 +1494,32 @@ def get_init_info(self) -> Dict[str, Any]:
This method provides the initialization info needed by the tokenizer manager
and other components to verify the scheduler is ready.
"""
# Fields below `max_req_input_len` are read by smg-grpc-servicer's
# GetModelInfo bridge (sglang/server.py:81-97, servicer.py:325-329)
# out of scheduler_info via .get(..., default). Without them the
# gateway sees defaults — supports_vision in particular is always
# False, which makes multimodal workers invisible to image/audio
# routing. They are no-ops for HTTP-mode consumers.
hf_cfg = self.model_config.hf_config
pad_id = getattr(hf_cfg, "pad_token_id", None)
bos_id = getattr(hf_cfg, "bos_token_id", None)
# `hf_eos_token_id` is annotated Optional[Set[int]] but the
# producer accepts int/list/None inputs; coerce defensively.
eos_ids = self.model_config.hf_eos_token_id
eos_token_ids = sorted([eos_ids] if isinstance(eos_ids, int) else eos_ids or [])
result_dict = {
"status": "ready",
"max_total_num_tokens": self.max_total_num_tokens,
"max_req_input_len": self.max_req_input_len,
"is_generation": self.is_generation,
"supports_vision": self.model_config.is_multimodal,
"vocab_size": self.model_config.vocab_size,
"eos_token_ids": eos_token_ids,
# int32 proto field; coerce missing IDs to the smg-grpc-servicer
# int defaults (0 for pad, 1 for bos) so encoding doesn't crash
# on `None`.
"pad_token_id": pad_id if pad_id is not None else 0,
"bos_token_id": bos_id if bos_id is not None else 1,
}
Comment on lines +1503 to 1523

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.

high

The logic for eos_token_ids has two issues:

  1. If hf_eos_token_id is a single integer (common in many models), sorted() will raise a TypeError as integers are not iterable.
  2. The use of or [] will treat a token ID of 0 as falsy and return an empty list instead of [0].

It's safer to explicitly handle the integer case and check for None.

References
  1. Defensive programming: ensure appropriate handling of different types (int vs list) and edge cases (0 as a valid ID).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the careful read. Both concerns are technically defused by an upstream invariant, but you're right that the diff doesn't show that — so I've pushed a defensive rewrite in 83c2f89 that makes the safety locally obvious.

Details on the original code, for the record:

  1. Bare int → TypeError — model_config.hf_eos_token_id is populated by ModelConfig._get_hf_eos_token_id, which normalizes HF's int | list[int] | None into a Set[int] (line 1307 wraps a bare int with {eos_ids}, line 1309 substitutes set() for None). By the time sorted() sees it, it's always a set.

  2. or [] drops 0 — or was operating on the set, not a scalar. bool({0}) is True, so {0} or [] == {0} and sorted({0}) == [0]. The falsy-zero footgun is real for scalar int fields but not for sets.

That said, the producer's type annotation is Optional[Set[int]], which advertises a None return the function never actually produces. The handshake shouldn't depend on a cross-file invariant the type system doesn't enforce, so the new commit coerces inline:

eos_ids = self.model_config.hf_eos_token_id
eos_token_ids = sorted(
    [eos_ids] if isinstance(eos_ids, int) else eos_ids or []
)

isinstance(int) runs first so token ID 0 survives (isinstance(0, int) is True, [0] or [] == [0]). Same behavior today, robust against future refactors.


return result_dict
Expand Down
Loading