Repository navigation
fix(grpc): drop stale WatchLoadUpdateReq piggyback, bump sglang floor to 0.5.13 - #1850
Conversation
|
Warning Review limit reached
More reviews will be available in 48 minutes and 6 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesSglang dependency and load path
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the minimum required version of sglang to 0.5.13 in pyproject.toml and removes the deprecated WatchLoadUpdateReq import and load-forwarding logic from request_manager.py. This change aligns with newer sglang releases that utilize shared-memory load snapshots for data-parallel balancing. I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
… to 0.5.13
sglang #28408 (released in v0.5.14) removed WatchLoadUpdateReq and the
BatchTokenIDOutput.load / BatchStrOutput.load piggyback fields. smg's
gRPC request manager still imported WatchLoadUpdateReq and forwarded
batch_out.load on the output streaming hot path, so importing
smg_grpc_servicer.sglang against sglang>=0.5.14 fails with
ImportError: cannot import name 'WatchLoadUpdateReq' from
'sglang.srt.managers.io_struct' -- making gRPC mode unusable.
The piggyback path is dead code on every release smg still supports:
- 0.5.13+ : DataParallelController reads load from shared-memory
snapshots (#26348) and no longer registers a WatchLoadUpdateReq
handler, so the forwarded message has no consumer.
- 0.5.10-0.5.12 : piggyback was the only DPBudget feed, but those
releases predate shmem and smg no longer supports them (see below).
DP load balancing is unaffected on supported releases: both the shmem
writer (scheduler.py) and reader (data_parallel_controller.py) live in
sglang, which smg launches unchanged; the request_manager only replaces
TokenizerManager and was never on the load path once shmem landed.
Bump the optional sglang dependency floor 0.5.10 -> 0.5.13 so the
supported range matches the shmem era and the removed piggyback path is
not silently missed. /v1/loads (smg's get_loads) keeps working on 0.5.13+
via the GetLoadsReqInput request/response path that scheduler still
serves.
Refs: sgl-project/sglang#28408
Signed-off-by: Jun Liu <jun.c.liu@rakuten.com>
0e39075 to
64230cf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a26d323c0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| sglang = ["sglang>=0.5.10"] | ||
| # 0.5.13 is the first release with the shmem load snapshots that replaced | ||
| # the WatchLoadUpdateReq piggyback path (removed upstream in v0.5.14). | ||
| sglang = ["sglang>=0.5.13"] |
There was a problem hiding this comment.
Enforce the SGLang floor outside extras
This dependency floor only applies when users install smg-grpc-servicer[sglang]; the repo's source install paths still install grpc_servicer/ without that extra after SGLang is already present (for example scripts/ci_install_sglang.sh pins sglang[all]==0.5.12.post1, and scripts/installation/install-smg.sh force-reinstalls the servicer source into engine images). In those environments, dp_size>1 with total_tokens/total_requests still needs the WatchLoadUpdateReq feed that was removed, because pre-0.5.13 SGLang's data-parallel controller consumes only that request rather than shmem snapshots, so the new code silently runs with stale DP load state instead of enforcing the unsupported version.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Looks it's better to bump sglang version in CI?
ShangmingCai
left a comment
There was a problem hiding this comment.
Thank you for the fix. The change looks good to me.
Description
Problem
sglang PR sgl-project/sglang#28408 (released in v0.5.14) removed
WatchLoadUpdateReqfromsglang.srt.managers.io_structand the.loadpiggyback field fromBatchTokenIDOutput/BatchStrOutput.smg-grpc-servicer's gRPC request manager still importedWatchLoadUpdateReqand forwardedbatch_out.loadto the scheduler on the output streaming hot path. As a result, importingsmg_grpc_servicer.sglangagainst sglang ≥ 0.5.14 fails at import time:This makes gRPC mode entirely unusable on sglang 0.5.14+ — reported in sgl-project/sglang#28408 (comment); @ShangmingCai confirmed the interface is deprecated and the fix belongs in smg (comment).
Solution
Drop the stale piggyback path and align the supported sglang range with the shared-memory load-snapshot era.
The piggyback was the DP load feed only on the pre-shmem releases; on every release smg will support after this PR it is dead code:
DP load balancing is unaffected on supported releases: both ends of the shmem link live inside sglang, which smg launches unchanged — the writer is
scheduler.py(load_snapshot_writer), the reader isdata_parallel_controller.py(load_snapshot_reader.read_all()). smg'srequest_manageronly replacesTokenizerManagerand was never on the load path once shmem landed./v1/loads(smg'sget_loads) keeps working on 0.5.13+ via theGetLoadsReqInputrequest/response path the scheduler still serves (scheduler.pyhandle_get_loads_req), which I verified remains present in v0.5.14.Changes
request_manager.py: remove theWatchLoadUpdateReqimport and the dead load-forwarding block in_handle_batch_output.pyproject.toml: bumpsglangfloor0.5.10→0.5.13.Test Plan
ruff check+ruff format --checkon changed file — pass.python -m pytest grpc_servicer/tests—20 passed, 6 skipped.import smg_grpc_servicer.sglang.request_managernow imports cleanly (wasImportError).dp_size>1,load-balance-method: total_tokens: DP balancing still distributes across ranks (shmem, no regression).Checklist
ruff check/ruff format --checkpass on changed Python (no Rust changes in this PR)python -m pytest grpc_servicer/testspasses (20 passed, 6 skipped)Refs: sgl-project/sglang#28408