fix: bind _get_port to wildcard address to avoid cross-job port collisions (#520)#777
fix: bind _get_port to wildcard address to avoid cross-job port collisions (#520)#777lonexreb wants to merge 1 commit into
Conversation
…meta-pytorch#520) _get_port() bound on 'localhost' (127.0.0.1), but the returned port was later used on the FQDN interface (socket.gethostname()) by TCPStore. The OS treats those as separate bind scopes, so two concurrent training jobs on the same node could be handed the same port number — manifesting as "TCP client failed to connect/validate" timeouts. Switch to bind('', 0) and add listen(1) so the kernel reserves the port across every local interface until the socket is closed. This matches the existing correct pattern in monarch_executor._get_free_port. Test plan: tests/unit_tests/test_get_port.py - returns a valid TCP port - returned port is currently free on the wildcard interface - structurally binds to '' (regression for meta-pytorch#520) - calls listen() to fully reserve the port
|
Hi @lonexreb! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
meta-cla
Bot
added
the
CLA Signed
1 participant
Summary
Fixes #520.
_get_port()insrc/forge/controller/provisioner.pybound onlocalhost(127.0.0.1), but the returned port was later consumed on the FQDN interface (socket.gethostname()) byTCPStore. Linux treats those as separate bind scopes, so two concurrent training jobs on the same node could be handed the same port number — surfacing as:Switching to
bind(('', 0))pluslisten(1)makes the kernel reserve the port across every local interface until the socket closes, eliminating the collision window. This matches the already-correct pattern inmonarch_executor._get_free_port.provisioner.pytests/unit_tests/test_get_port.py(4 tests, all pass against the fix and fail against the prior impl)Test plan
test_returns_valid_tcp_port— returned value is an integer port in [1, 65536)test_returned_port_is_currently_free_on_wildcard— wildcard rebind on the returned port succeedstest_binds_to_wildcard_address_not_localhost— structural regression for forge can not run two training job at the same time in same node due to port 36121 not available #520 (assertsbind(('', 0)))test_calls_listen_to_reserve_port— structural regression (assertslisten()is called)apps/grpo/main.pylaunches on a single multi-GPU host no longer collide (cannot run locally — please verify in CI / on a GPU box)Notes
monarch_executor._get_free_portandprovisioner._get_portnow have nearly identical bodies. Happy to fold them into a shared helper in a follow-up if maintainers want, but kept this PR strictly scoped to the bug fix.