Skip to content

Centralize safe runtime port allocation - #156

Merged
weireweire merged 2 commits into
NVIDIA:mainfrom
weireweire:port-safe-range-preflight
May 15, 2026
Merged

weireweire merged 2 commits into
NVIDIA:mainfrom
weireweire:port-safe-range-preflight

Conversation

@weireweire

Copy link
Copy Markdown
Collaborator

Summary:

  • Revert per-job SGLang port jitter in favor of centralized low, non-ephemeral runtime port ranges.
  • Add preflight warnings for planned ports inside the kernel ephemeral range.
  • Adjust busy planned worker ports before launch and log the old and new values.
  • Update backend/frontend/infra defaults and related unit tests.

Validation:

  • python3 -m py_compile on changed Python files and targeted tests
  • bash -n src/srtctl/benchmarks/scripts/profiling/profile.sh
  • git diff --check

Not run:

  • pytest/ruff, because uv/pytest/ruff are not installed in this local environment and the system Python is too old for package runtime imports.

@weireweire
weireweire force-pushed the port-safe-range-preflight branch 5 times, most recently from 4ed2e83 to f2785f8 Compare May 14, 2026 07:59
@codecov-commenter

codecov-commenter commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.14286% with 10 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@9cd8fe8). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/srtctl/cli/setup_head.py 0.00% 6 Missing ⚠️
src/srtctl/cli/mixins/benchmark_stage.py 33.33% 2 Missing ⚠️
src/srtctl/cli/mixins/worker_stage.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #156   +/-   ##
=======================================
  Coverage        ?   65.04%           
=======================================
  Files           ?       67           
  Lines           ?     8207           
  Branches        ?        0           
=======================================
  Hits            ?     5338           
  Misses          ?     2869           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@weireweire
weireweire force-pushed the port-safe-range-preflight branch from f2785f8 to 70872c4 Compare May 14, 2026 08:22
@weireweire
weireweire force-pushed the port-safe-range-preflight branch from 70872c4 to fa6c2f3 Compare May 14, 2026 08:41
@weireweire
weireweire marked this pull request as ready for review May 14, 2026 09:41
@weireweire

Copy link
Copy Markdown
Collaborator Author

tested on mars and sglang and passed.

@weireweire
weireweire merged commit 38ceec4 into NVIDIA:main May 15, 2026
6 checks passed
esmeetu added a commit to esmeetu/srt-slurm that referenced this pull request May 15, 2026
Resolve conflicts from PR NVIDIA#156 (centralized port allocation) which
moved port constants out of per-backend modules.

- Delete src/srtctl/backends/mooncake.py (constants now live in ports.py)
- Add MOONCAKE_METRICS_PORT (8702) to srtctl/ports.py alongside the
  existing MOONCAKE_MASTER_PORT (8700) and MOONCAKE_HTTP_METADATA_PORT
  (8701) so all three live in the same 8700-range
- Repoint imports in sglang.py, vllm.py, do_sweep.py, submit.py
- Refresh docs/mooncake-kv-store.md and CLAUDE.md to use the new ports
- Update test_dry_run.py and test_e2e.py to import port constants from
  srtctl.ports instead of hardcoding 50051

Workers now see MOONCAKE_MASTER=<infra>:8700 (was 50051) and
MOONCAKE_TE_META_DATA_SERVER=http://<infra>:8701/metadata. The
mooncake_master srun is launched with explicit --port, --http_metadata_server_port,
and --metrics_port flags so we don't rely on mooncake's compile-time defaults.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants