[https://nvbugs/6567057][infra] Unwaive disagg helix test after port reservation race fix - #17868
Conversation
|
/bot run |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughUpdates ChangesIntegration waiver maintenance
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change only removes a stale test waiver, and no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #66986 [ run ] triggered by Bot. Commit: |
|
PR_Github #66986 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #67239 [ run ] triggered by Bot. Commit: |
…reservation race fix
The waiver was added during post-merge 2887 triage, when trtllm-serve's
port reservation was subject to a TOCTOU race: get_free_ports() bound and
closed a probe socket, so another process could take the port before the
real bind. The stage log shows the resulting failure:
[_utils] [get_free_ports] pid=3520683 reserved ports=[58059] via
bind-then-close (subject to TOCTOU reuse before rebinding)
OSError: [Errno 98] Address already in use
RuntimeError: Failed to bind socket to localhost:58059: [Errno 98]
Address already in use. Port holder(s): 127.0.0.1:58059
status=LISTEN pid=3532210 name=trtllm-serve
That race was eliminated by commit 9997d3f, which switches the server
to --port 0 + --report_addr so the socket is held until the real bind.
NVBug 6567057 is closed as a duplicate of 6435121, so the waiver is stale
and the test can run in CI again.
Since the waiver was written, the test was split into -overlap_on and
-overlap_off parametrizations, so both waiver lines are removed. In
l0_dgx_b200.yml (8-GPU B200, pytorch/mpi) -overlap_on is tagged
stage: pre_merge and -overlap_off is stage: post_merge, so the standard
pipeline exercises the -overlap_on variant.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
ee64209 to
3bc5eb1
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #67246 [ run ] triggered by Bot. Commit: |
|
PR_Github #67239 [ run ] completed with state |
|
PR_Github #67246 [ run ] completed with state |
4 participants
Description
Removes a stale waiver from
tests/integration/test_lists/waives.txt:The waiver was added in #17340 during post-merge 2887 triage, when
trtllm-serve's portreservation was subject to a TOCTOU race:
get_free_ports()bound and closed a probe socket,so another process could take the port before the real bind. The stage log for that failure shows:
That race was eliminated by 9997d3f (#17460), which switches the server to
--port 0+--report_addrso the socket is held until the real bind. NVBug 6567057 is closed as aduplicate of 6435121 (the bug that 9997d3f fixes), so the waiver outlived its cause.
Test Coverage
accuracy/test_disaggregated_serving.py::TestDeepSeekV3Lite::test_auto_dtype_with_helix[fifo_v2-cudagraph:with_padding-pp1dp2cp2]Tagged
stage: pre_mergeinl0_dgx_b200.yml(8-GPU B200, pytorch/mpi), so the standardpipeline exercises it — no
--extra-stageneeded.PR Checklist
[JIRA/NVBUG/None][type] SummaryDev Engineer Review
TestDeepSeekV3Lite::test_auto_dtype_with_helixwithfifo_v2-cudagraph:with_padding-pp1dp2cp2.tests/integration/test_lists/waives.txt.trtllm-serveport reservation race.QA Engineer Review
test-db/orqa/files.tests/integration/test_lists/waives.txt.stage: pre_mergeconfiguration inl0_dgx_b200.yml.