fix(distributed): bind a free worker port before spawning - #1166
fix(distributed): bind a free worker port before spawning#1166burak-fal wants to merge 2 commits into
Conversation
Follow-up: real GPU lifecycle verification on
|
| Scenario | Observed result |
|---|---|
| Two groups starting concurrently in one allocation | Distinct worker ports 33021 / 45489, independent all_reduce results 20 / 30, four NCCL ranks in each group |
| Intentional rank-0 setup failure | No surviving children, failed port 33431 could be rebound, same object restarted and returned 20 |
| Startup cancellation | Cancellation propagated, no surviving children, cancelled port 41789 could be rebound before probe cleanup, recovery returned 20 |
| Normal restart after those cases | Result 30, worker port 46859 → 35097, master stayed 29500 |
| Explicit worker port 55001 | Requests returned 20 / 30 and the port was retained across restart |
| Explicit occupied worker port 54923 | Expected Address already in use startup failure, no silent fallback |
Scope note: the concurrent groups used explicitly distinct master ports 29500 / 29501. This PR does not change or claim to solve automatic master-port allocation. Production diff remains one file, +38 / -16 lines. No unit-test files were added.
All four GPU allocations from this follow-up were confirmed TERMINATED through the owner-scoped runner API.
App source and actual terminal evidence, especially LIFECYCLE_RESULTS.txt, 04-cancel-start-before-fix.log, and 06-cancel-start-after-fix.log. The evidence retains the failed attempt and the actual development-build metadata.
Readiness boundary
Real GPU acceptance, Ruff, and the focused mypy check passed. GitHub CI is not fully green: 15 passed, 14 integration/e2e checks failed, 1 skipped. The same base commit already had failing integration and e2e suites before this branch, with overlapping timeout and exec failure types. No unrelated tests, timeouts, or workflows were weakened to hide those failures.
The PR remains draft for maintainer review and CI-context assessment. No merge, SDK release, or customer rollout has been performed.
Why
The default ZMQ worker port can already be occupied inside the same runtime, including by Isolate's local agent. Starting a DistributedRunner should not require the app to coordinate that internal port.
Refs #1165. This fixes the worker-port collision only, not the separate master-port behavior discussed there.
Change
worker_porttoNoneand bind the parent ROUTER withbind_to_random_port()before spawning workers.self.runis serialized. Children receive the resolved port number and connect to it, rather than binding it again. There is no find-free-port / close / rebind window.master_port=29500and the NCCL rendezvous path are unchanged.One production file, +38 / -16 lines. No backend or infrastructure changes.
Initial real GPU verification (September 10)
Exercised private
fal.Appinstances throughfal runand real HTTP requests on four NVIDIA H100 80GB GPUs, with PyTorch 2.4.1 and NCCL. The probe keeps TCP port 54923 occupied throughout the run and performs a GPUall_reduceovervalue * (rank + 1).fal==1.80.0, default worker portAddress already in useon127.0.0.1:54923, startup failed, CLI exit 1nccl, worker port 45927Address already in use, CLI exit 1. No silent fallbackMaster port stayed 29500 in every successful response. All five GPU runtime runners, including an initial probe correction attempt, were confirmed
TERMINATEDthrough the owner-scoped runner API after the checks.Probe source, launcher, recorded terminal outputs, and result notes. The notes retain the initial probe callback error and the actual development-build version reported by the remote environment.
Ruff 0.3.4 formatting/lint and the configured focused mypy 1.3.0 check passed. No unit tests were added and no local pytest suite was run. Existing GitHub CI runs separately. The probe and terminal artifacts stay outside this SDK diff.
Final-head lifecycle verification (September 11)
Additional real four-H100 tests found a cancellation cleanup gap: cancelling
start()after spawn propagatedCancelledError, but left four children and the bound worker port alive. Commit55808a33adds three lines to run the existing termination cleanup and re-raise cancellation.Retested the final code with these results:
All four additional GPU allocations were confirmed
TERMINATEDthrough the owner API. The evidence gist includeslifecycle_app.py,LIFECYCLE_RESULTS.txt, and both the failed pre-fix cancellation output and successful final-head outputs. No unit-test files were added. Master-port behavior remains unchanged.CI status
Final head
55808a33is not fully green: 15 checks passed, 14 integration/e2e matrix checks failed, and one check was skipped. Pre-commit, the unit matrix, CodeQL, and Socket checks passed. Ruff and the focused configured mypy check also passed locally.The final-head integration and e2e jobs show general 60-second timeouts and the long-exec assertion failure. These suites do not reference
DistributedRunner. The same base commit (b5070518) already had failing scheduled integration and e2e runs before this branch, with overlapping failure types. This is not a claim that every backend failure has been diagnosed or that CI is green.No unrelated tests or CI timeouts were changed or weakened. Keeping the PR draft for maintainer review. Merge, SDK release, and customer rollout remain outside this verification run.