[https://nvbugs/6143883][fix] Preserve ip:port for trtllm-serve visual-gen - #14355
Conversation
📝 WalkthroughWalkthroughThe ChangesSocket pre-binding in Visual Gen server
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
7677e22 to
ff5e568
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #49403 [ run ] triggered by Bot. Commit: |
|
PR_Github #49403 [ run ] completed with state
|
|
/bot run |
|
PR_Github #49561 [ run ] triggered by Bot. Commit: |
|
PR_Github #49561 [ run ] completed with state
|
ff5e568 to
80f04cb
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #49655 [ run ] triggered by Bot. Commit: |
|
PR_Github #49655 [ run ] completed with state
|
80f04cb to
8b00ca6
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #50317 [ run ] triggered by Bot. Commit: |
|
PR_Github #50317 [ run ] completed with state
|
|
/bot run |
|
PR_Github #51513 [ run ] triggered by Bot. Commit: |
|
PR_Github #51513 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #52272 [ run ] triggered by Bot. Commit: |
|
PR_Github #52272 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #52363 [ run ] triggered by Bot. Commit: |
|
PR_Github #52363 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #52420 [ run ] triggered by Bot. Commit: |
|
PR_Github #52420 [ run ] completed with state
|
|
/bot run |
|
PR_Github #52638 [ run ] triggered by Bot. Commit: |
|
PR_Github #52638 [ run ] completed with state
|
|
/bot run |
|
PR_Github #52726 [ run ] triggered by Bot. Commit: |
|
PR_Github #52726 [ run ] completed with state
|
…l-gen VisualGen model initialization can take many minutes between the test-side free-port check and uvicorn's bind() call. During that window anything else on the host can grab the port, and trtllm-serve dies at bind() time -- which the visual-gen e2e fixture surfaces as "RuntimeError: Visual-gen server exited unexpectedly." Fix: - launch_visual_gen_server now binds the listening socket *before* constructing VisualGen, then hands the bound socket to uvicorn via the existing OpenAIServer.__call__(..., sockets=[s]) path, collapsing the check-to-bind window to a few syscalls. - RemoteVisualGenServer in the visual-gen e2e test now allocates its port via get_free_port_in_ci, so parallel pytest sessions on the same OCI node fall into disjoint container port sections (CONTAINER_PORT_START / CONTAINER_PORT_NUM); falls back to the plain free-port scan when those env vars are not set. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
The previous commit accidentally renamed three attribute accesses on VisualGenArgs: - args.parallel -> should be args.parallel_config - .dit_cfg_size -> should be .cfg_size - .dit_ulysses_size -> should be .ulysses_size These look like a rebase artifact from a parallel branch where the field was temporarily renamed; main still exposes the original names (see tensorrt_llm/visual_gen/args.py: VisualGenArgs.parallel_config, ParallelConfig.cfg_size / ulysses_size / n_workers). The mismatch caused the visual-gen test to fail in CI: workers came up successfully, then immediately after `Workers ready` the next line `args.parallel.n_workers` raised AttributeError. The exception escaped the `with socket(...)` block, the socket was torn down before uvicorn ever bound it, and the test saw "Visual-gen server exited unexpectedly" / connection refused on /health. The socket pre-bind logic itself is unchanged and matches the agg-LLM (serve.py L313-360) and disagg (serve.py L1245-1276) patterns, which also bind without an explicit s.listen() — uvicorn / asyncio's loop.create_server(sock=...) calls listen() internally. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
86cb02f to
b97e8bd
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #52916 [ run ] triggered by Bot. Commit: |
|
PR_Github #52916 [ run ] completed with state
|
|
/bot run |
|
PR_Github #53019 [ run ] triggered by Bot. Commit: |
|
PR_Github #53019 [ run ] completed with state
|
|
/bot run |
|
PR_Github #53247 [ run ] triggered by Bot. Commit: |
|
PR_Github #53247 [ run ] completed with state |
The L0 Post-Merge test
unittest/_torch/visual_gen/test_trtllm_serve_e2e.py::TestWanTextToVideo::test_health flakes on DGX_B200 with "RuntimeError: Visual-gen server exited unexpectedly." Root cause is the same OCI port-conflict race that already hit the agg-LLM and disagg paths: there is a window between the test-side get_free_port() check and uvicorn's bind() inside trtllm-serve during which another container can grab the port. The visual-gen server makes the window much wider because VisualGen model initialization can take many minutes before uvicorn ever attempts to bind, so any colliding process almost always wins.
Apply the same socket-pre-binding pattern already adopted for the agg LLM server (nvbugs/5703953, PR #9646) and the disagg server (nvbugs/5727517, PR #9859): bind the listening socket before constructing VisualGen, then hand the bound socket to uvicorn via the existing OpenAIServer.call sockets= parameter. This collapses the check-to-bind window from "model load time" to "a few syscalls".
Summary by CodeRabbit
Bug Fixes
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.