Fix SGLang sidecar launch and endpoint integration - #362
Merged
ishandhanani merged 4 commits intoSep 3, 2026
Merged
Conversation
Root cause: sidecar mode launches native sglang.launch_server, but the worker command still appended Dynamo's --dump-config-to argument. Native SGLang rejects that argument during startup.\n\nFix: gate the dump argument on the existing native-SGLang selection so only dynamo.sglang receives it. Add a sidecar command regression assertion.\n\nValidation: 8 sidecar-focused tests passed; Ruff check and format passed for the changed files. Signed-off-by: weireweire <20922698+weireweire@users.noreply.github.com>
weireweire
requested review from
alec-flowers,
csahithi,
ishandhanani and
nlevin-ui
as code owners
August 28, 2026 07:39
Root cause: the Dynamo SGLang sidecar defaults its native gRPC readiness deadline to 300 seconds, while srt-slurm permits much longer model startup through health_check. Large models can still be loading when the sidecar exits and tears down the engine. Fix: derive one timeout from health_check.max_attempts and health_check.interval_seconds, pass it to the sidecar as --health-deadline-secs in both Slurm and direct launch paths, and reuse the same value for direct health polling. Validation: 230 configuration, direct-plan, and direct-runner tests passed; Ruff source checks and format checks passed. Signed-off-by: weireweire <20922698+weireweire@users.noreply.github.com>
Root cause: SGLang sidecar mode was still classified as an in-process Dynamo worker when selecting logical metrics and control endpoints. This advertised Dynamo system ports, which do not expose the native SGLang metrics or control APIs. Fix: keep system ports for in-process Dynamo, but select each leader's native SGLang HTTP port for sidecar mode. Add coverage for disaggregated prefill/decode endpoint and metrics URL generation. Validation: uv run pytest tests/test_benchmarks.py::TestCustomBenchmarkRunner -q
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #362 +/- ##
=======================================
Coverage ? 72.64%
=======================================
Files ? 99
Lines ? 13656
Branches ? 0
=======================================
Hits ? 9921
Misses ? 3735
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Main's sidecar rearchitecture (NVIDIA#363) superseded this branch's original fixes (mixin-level _wrap_sglang_sidecar/uses_sidecar/engine_mode), so most of this merge drops that code in favor of main's backends/sidecar.py + sglang.py implementation. Ported forward the one still-relevant fix: _logical_worker_endpoints was still advertising Dynamo system ports for sidecar-mode workers instead of native SGLang HTTP ports, which don't expose Dynamo's metrics/control APIs. Adapted to main's config.dynamo.sidecar field, with regression test.
ishandhanani
force-pushed
the
fix/sidecar-dump-config-argument
branch
from
September 2, 2026 19:00
ca78d25 to
c70cd34
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--dump-config-toto nativesglang.launch_serverin sidecar modehealth_check.max_attempts * health_check.interval_secondsWhy
Sidecar mode introduced in #354 launches native SGLang and connects it to Dynamo through the standalone sidecar. Three integration details prevented existing workloads from completing cleanly:
--dump-config-to, which is accepted bydynamo.sglangbut rejected by nativesglang.launch_server.The sidecar now receives
--health-deadline-secsfrom the same health-check budget used by srt-slurm and exits early as soon as native gRPC becomes ready. In-process Dynamo keeps using system ports, while SGLang sidecars use their native leader HTTP ports.Validation