Repository navigation
fix(rust-server): Drain recv_from_rpc in rust-server mode - #33516
MortalHappiness wants to merge 2 commits into
Conversation
Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
|
||
| # The rpc channel is a bidirectional zmq DEALER pair with the |
There was a problem hiding this comment.
Is this AI-generated code? Too many comments...
There was a problem hiding this comment.
I'll shorter the comments and make the code more self-explantory. Thanks!
Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
|
@MortalHappiness , Let's review the scenarios that has to send request via rpc, and double check whether we can migrate to rust. |
|
Thanks @MortalHappiness. Closing this because it is still a draft and has not been updated in 52 days. Reopen it if the work is still relevant. Some directories moved recently, so an older branch may need retargeting: |
Motivation
Closes the "recv_from_rpc is never drained in rust-server mode" TODO item from #29799.
Rank 0's scheduler drains two input channels per event-loop iteration: the tokenizer channel (zmq PULL,
scheduler_input_ipc_name) and the rpc channel (zmq DEALER,rpc_ipc_name, used only by the offlinesgl.Engine'scollective_rpc).SGLANG_RUST_SERVERreplaces the first with an in-process ring and has no rpc equivalent, so the second has to keep working in both modes.The rust branch was added at the top of
_pull_raw_reqsand ends inreturn, which exits the whole function: it bypasses the tokenizer drain (intended) and the rpc drain below it (not intended). The socket is still created and wired into the receiver, but nothing reads it. The reply direction has the mirror problem:process_input_requeststestedself.rust_server is not Nonebeforeisinstance(output, RpcReqOutput), so the zmq reply branch was unreachable, and the Rust egress path it took instead asserts on a rust-minted rid that an rpc request arriving over zmq does not carry.Modifications
In
scheduler_components/request_receiver.py, the earlyreturnbecomes anif/else, so rust mode bypasses only the tokenizer drain. The rpc loop body is unchanged.In
scheduler.py,process_input_requestsnow testsisinstance(output, RpcReqOutput)before therust_serverbranch, so an rpc reply goes back down the DEALER pair instead of into the Rust egress ring. This is behaviour-preserving in zmq mode: thererust_serverisNone, so that branch was already reached.The two are one fix: draining without the reorder would make the first rpc request in rust mode hit the rid assert and take the scheduler down, turning a silent hang into a crash.
Deliberately not changed
flumeexposes no fd andzmq.Pollercannot watch the ring. The delay is bounded and documented in a comment at the drain, and it is negligible for the checkpoint exports that are its only in-tree callers.Accuracy Tests
N/A
Speed Tests and Profiling
N/A
Checklist
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): ❌ Run #30952583076
Latest PR Test (Extra): ❌ Run #30952582600