CI: fix SGLang router port collision in sanity test - #1995
Conversation
get_next_tcp_port() checks ss -tuln at call time but does not bind the port, so PROXY_PORT allocated at startup (before the servers run) is vulnerable to a race: SGLang binds internal gRPC/ZMQ ports derived from its base port after startup, and one of those can land on PROXY_PORT before sglang_router tries to bind it. The router then exits immediately with "Address already in use", and the masked `|| true` on the health wait lets the script limp on until the 120s request timeout fires. Fix: re-allocate PROXY_PORT after both SGLang servers pass their health checks, so get_next_tcp_port() sees all internally-bound ports in ss -tuln and skips past them. Also remove the `|| true` so a router startup failure surfaces immediately instead of 120s later. Reported by the CI triage agent on PR ai-dynamo#1987. Signed-off-by: Daniel Pressler <danielpr@nvidia.com>
|
👋 Hi dpressle! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
|
/build |
📝 WalkthroughWalkthroughThe SGLang sanity script reallocates the proxy port after prefill/decode servers start, launches the router on that port, and treats router health-check failures as fatal. ChangesSGLang sanity flow
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitlab/test_vllm_sglang_sanity.sh:
- Line 160: Update get_next_tcp_port in common.sh to revalidate each candidate
after wrapping from tcp_port_max to tcp_port_min, continuing to scan for an
available port rather than returning an occupied one. If the entire configured
range is occupied, report port allocation exhaustion instead of returning a
colliding value; keep PROXY_PORT’s existing use unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 8ffe175d-8fc3-4432-b10d-4e95e0fe1f8d
📒 Files selected for processing (1)
.gitlab/test_vllm_sglang_sanity.sh
|
🤖 CI Triage Agent — TL;DR: The build succeeded; the job failed because the Full analysisSummary: Stage "Allocate DL EP Environment" (#200) failed at Root cause: SLURM resource allocation on Implicated commit: none — not caused by commit 3587eca or PR #1995. The failure is environmental (SLURM cluster capacity + Jenkins agent disconnect). File: N/A — failure is in the Jenkins/SLURM allocation step ( Suggested fix: Retry the build — this is a transient infrastructure failure. If it recurs: (1) check Related: none |
3 participants
get_next_tcp_port() checks ss -tuln at call time but does not bind the port, so PROXY_PORT allocated at startup (before the servers run) is vulnerable to a race: SGLang binds internal gRPC/ZMQ ports derived from its base port after startup, and one of those can land on PROXY_PORT before sglang_router tries to bind it. The router then exits immediately with "Address already in use", and the masked
|| trueon the health wait lets the script limp on until the 120s request timeout fires.Fix: re-allocate PROXY_PORT after both SGLang servers pass their health checks, so get_next_tcp_port() sees all internally-bound ports in ss -tuln and skips past them. Also remove the
|| trueso a router startup failure surfaces immediately instead of 120s later.Reported by the CI triage agent on PR #1987.
What?
Describe what this PR is doing.
Why?
Justification for the PR. If there is an existing issue/bug, please reference it. For
bug fixes, the 'Why?' and 'What?' can be merged into a single item.
How?
It is optional, but for complex PRs, please provide information about the design,
architecture, approach, etc.
Summary by CodeRabbit