Repository navigation
support rust sglang server - #29799
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
f2dbfc6 to
02bb343
Compare
|
Need to fix: File "/sgl-workspace/sglang/python/sglang/srt/managers/scheduler_components/request_receiver.py", line 93, in recv_requests
recv_reqs = self._broadcast_reqs_across_ranks(recv_reqs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/sgl-workspace/sglang/python/sglang/srt/managers/scheduler_components/request_receiver.py", line 209, in _broadcast_reqs_across_ranks
recv_reqs = broadcast_pyobj(
^^^^^^^^^^^^^^^^
File "/sgl-workspace/sglang/python/sglang/srt/utils/common.py", line 1474, in broadcast_pyobj
serialized_data = pickle.dumps(data)
^^^^^^^^^^^^^^^^^^
TypeError: cannot pickle 'mappingproxy' object |
ishandhanani
left a comment
There was a problem hiding this comment.
Reviewed the current head as the landed Rust /generate + streaming and tokenizer/detokenizer core described in the PR and RFC. I intentionally excluded the documented follow-up work: encoder/multimodal, PD/DP routing, metrics/logging, OpenAI/control/auth/API parity, standalone/multiprotocol, metadata, drain alignment, release/CI, and all performance findings.
The unary and SSE happy paths work, cargo fmt --check passes, and all 16 Rust unit tests pass. The latest commit also correctly moves logprob-text decoding from repeated cumulative API snapshots to once per detokenizer delta.
The inline comments are correctness and lifecycle issues reproduced on the current head: the Python fallback cannot launch, landed logprob inputs can terminate the scheduler, bounded handoffs can drop output or leak request state, listener/model readiness is not synchronized, slow streams can truncate while emitting [DONE], and stop/error semantics diverge. These should be fixed before the Rust path is enabled beyond development use.
74f99ba to
f2bbfdf
Compare
alexnails
left a comment
There was a problem hiding this comment.
Fable review incoming. Its comments LGTM, but resolve those that do not fit.
There was a problem hiding this comment.
Nice to see the rust frontend path taking shape. One thing I found hard to follow is where the protocol contracts are meant to live. Some user-facing openai paths use dynamo_protocols, some build json directly (openai.rs), and the frontend-to-scheduler uses msgpack. Maybe this is a good opportunity to clarify the contract for both internal communication and external callers?
The dynamo_protocols support the reasoning and function call (tool usage). The openai.rs is just a http api wrapper for the chat/completions api base on the dynamo_protocol. |
e534244 to
aa6a0d2
Compare
ishandhanani
left a comment
There was a problem hiding this comment.
Full current-head rereview at aa6a0d238b against exact base abafe0022e. Recommendation: request changes. I reviewed all 33 changed files and ran live GPU behavior, exact base-vs-head compatibility checks, CPU-only lifecycle/affinity fault injection, A/B/B/A load, and independent correctness/lifecycle/performance passes.
Validation: cargo fmt --check passed; cargo test --all-targets --locked passed 36/36; clippy with -D warnings failed on the large enum variants and one test-only lookup. Live testing used Qwen/Qwen3-0.6B on 1x L40S. Unary generation, normal SSE, prompt logprobs, malformed-logprob isolation, over-context unary 400, /health, and /health_generate passed. The earlier invalid-ingress leak is fixed: 10,000 invalid requests all returned 400 with 3,032 KiB scheduler RSS growth.
The performance A/B/B/A run was inconclusive: current-head runs differed by 13.8%, while one base phase had 16 approximately five-second tails. I am not treating it as evidence of either a win or a regression.
The inline comments below contain the reproduced or source-proven blockers. Explicitly documented future-work items were excluded.
9ba90ae to
1750ee9
Compare
1750ee9 to
128e533
Compare
e58d7a1 to
2b43543
Compare
Motivation
RFC #23206
Modifications
Splite PRs:
Done
Pending
Landed
input_idsbuffer, msgpackscalar header); logprob/hidden request params plumbed;
is_health_checkflagso the scheduler skips probes without occupying a queue slot (rids stay
u64).push_batch) — N per-requestmsgpack encodes + FFI crossings collapsed to one; logprobs (output/input,
top-k, token-ids), hidden states,
return_text_in_logprobs./generate(unary + SSE),/server_info,/get_model_info(+
/model_info),/v1/models,/health+/health_generate.RustServerIdleSleeper) insteadof spinning at 100% CPU; zero added latency (ring wakes on push).
acceptskips detok/meta_info dead-work in rust mode(only the fields the Rust egress reads).
TODO / Open
tok/detok is offloaded — single Python thread;
accept/add_one_req_ignore_eos/process_batch_resultdominate. Needs per-stepwork reduction / further offload.
/v1/completions,/v1/chat/completions) — requiredfor
lm_eval; currently only SGLang-native/generate.so the egress is a single
.tobytes()instead of a per-request flatten.meta_infoparity gaps:cached_tokens,reasoning_tokens, spec-decodestats, etc. not yet emitted by the Rust egress.
SGLANG_SCHEDULER_MAX_RECV_PER_POLL=-1should mean"drain all" (matches zmq); rust
draincurrently caps at 256 — decide/align.dp_size > 1support (currently rank-0 embedded only).Accuracy Tests
Speed Tests and Profiling
Checklist
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): ✅ Run #30587617646
Latest PR Test (Extra): ✅ Run #30598381601