[train] set Neuron rendezvous env vars in the XLA backend - #65071
[train] set Neuron rendezvous env vars in the XLA backend#65071violivei wants to merge 1 commit into
Conversation
fd6adc8 to
47371b7
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces support for configuring the network interface and allocating a dedicated rendezvous port for the Neuron runtime in TorchXLAConfig. Specifically, it adds a socket_ifname parameter and sets the NEURON_RT_ROOT_COMM_ID, NCCL_SOCKET_IFNAME, and CCOM_SOCKET_IFNAME environment variables. The feedback highlights two main improvements: first, sequentially calling get_address_and_port may result in port conflicts, so both ports should be allocated simultaneously in a single remote call; second, variables like root_comm_port and backend_config should be passed explicitly to set_env_vars rather than captured in a closure to avoid serialization issues.
| # Get master address and port from the first worker. | ||
| master_addr, master_port = worker_group.execute_single(0, get_address_and_port) | ||
|
|
||
| # The Neuron runtime rendezvous cannot share MASTER_PORT with | ||
| # torch.distributed, so take a second free port on the first worker. | ||
| _, root_comm_port = worker_group.execute_single(0, get_address_and_port) |
There was a problem hiding this comment.
Calling get_address_and_port twice sequentially on the same worker can easily return the exact same port because the first port is closed before the second call binds. This would cause master_port and root_comm_port to conflict, leading to bind errors during training.
Additionally, executing two separate synchronous remote tasks (execute_single) adds unnecessary round-trip latency. We can solve both issues by executing a single remote function that binds two sockets simultaneously to guarantee distinct ports.
# Get master address and two free ports from the first worker.
# The Neuron runtime rendezvous cannot share MASTER_PORT with
# torch.distributed, so we must allocate two distinct ports.
# We allocate them simultaneously to guarantee they are distinct.
def get_ports():
import socket
from ray.train._internal.utils import is_ipv6
addr = ray.util.get_node_ip_address()
family = socket.AF_INET6 if is_ipv6(addr) else socket.AF_INET
with socket.socket(family, socket.SOCK_STREAM) as s1, socket.socket(family, socket.SOCK_STREAM) as s2:
s1.bind(("", 0))
s2.bind(("", 0))
return addr, s1.getsockname()[1], s2.getsockname()[1]
master_addr, master_port, root_comm_port = worker_group.execute_single(0, get_ports)| def set_env_vars(addr, port): | ||
| os.environ["MASTER_ADDR"] = addr | ||
| os.environ["MASTER_PORT"] = str(port) | ||
| # To trigger the xrt server | ||
| os.environ["TORCHELASTIC_RUN_ID"] = self.unique_run_id | ||
| # Rendezvous endpoint the Neuron runtime uses to build a communicator | ||
| # spanning instances. Unset, each rank builds its own local | ||
| # communicator and a multi-instance collective never forms. | ||
| os.environ["NEURON_RT_ROOT_COMM_ID"] = build_address(addr, root_comm_port) | ||
| ifname = backend_config.socket_ifname | ||
| if ifname: | ||
| # CCOM chooses its bootstrap socket by looking for a local | ||
| # interface whose subnet contains the peer address. That search | ||
| # cannot succeed where hosts carry a /32, so allow the interface | ||
| # to be named outright. | ||
| os.environ.setdefault("NCCL_SOCKET_IFNAME", ifname) | ||
| os.environ.setdefault("CCOM_SOCKET_IFNAME", ifname) | ||
|
|
||
| # Set the env vars on all workers. | ||
| worker_group.execute(set_env_vars, addr=master_addr, port=master_port) |
There was a problem hiding this comment.
Instead of implicitly capturing root_comm_port and backend_config in the set_env_vars closure, it is cleaner and more robust to pass them explicitly as arguments. This avoids potential serialization issues with larger objects in remote worker tasks.
def set_env_vars(addr, port, root_comm_port, socket_ifname):
os.environ["MASTER_ADDR"] = addr
os.environ["MASTER_PORT"] = str(port)
# To trigger the xrt server
os.environ["TORCHELASTIC_RUN_ID"] = self.unique_run_id
# Rendezvous endpoint the Neuron runtime uses to build a communicator
# spanning instances. Unset, each rank builds its own local
# communicator and a multi-instance collective never forms.
os.environ["NEURON_RT_ROOT_COMM_ID"] = build_address(addr, root_comm_port)
if socket_ifname:
# CCOM chooses its bootstrap socket by looking for a local
# interface whose subnet contains the peer address. That search
# cannot succeed where hosts carry a /32, so allow the interface
# to be named outright.
os.environ.setdefault("NCCL_SOCKET_IFNAME", socket_ifname)
os.environ.setdefault("CCOM_SOCKET_IFNAME", socket_ifname)
# Set the env vars on all workers.
worker_group.execute(
set_env_vars,
addr=master_addr,
port=master_port,
root_comm_port=root_comm_port,
socket_ifname=backend_config.socket_ifname,
)There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 47371b7. Configure here.
_TorchAwsNeuronXLABackend never exports NEURON_RT_ROOT_COMM_ID, so the Neuron runtime has no endpoint at which ranks on different instances can meet. Every rank builds its communicator locally and startup fails in Nrt::BuildGlobalComm. A single instance has nothing to rendezvous with, so the gap only shows once a job crosses a host boundary. on_start already asks worker 0 for get_address_and_port and uses the result for MASTER_ADDR and MASTER_PORT. Replace that call with _get_address_and_ports, which returns the same address plus two free ports, and publish address and second port as NEURON_RT_ROOT_COMM_ID to every worker. The helper holds both sockets bound until it has read both port numbers, so the two ports cannot be equal. Calling get_address_and_port twice would not give that guarantee, because find_free_port closes its socket before returning and the port is back in the pool before the next probe. Add an optional socket_ifname field to TorchXLAConfig. Neuron's CCOM layer picks its bootstrap socket by looking for a local interface whose subnet contains the peer address, which cannot succeed where hosts carry a /32, as pods do under the AWS VPC CNI. Naming the interface skips the subnet search. The field defaults to None so nothing changes for callers who do not opt in, and the backend uses setdefault so an interface name the user has already exported wins. The set_env_vars closure takes the root comm port and the interface name as arguments rather than capturing them, so it does not close over the backend config. Signed-off-by: Victor Oliveira Antonino <7117999+violivei@users.noreply.github.com>
|
Thanks for the review, both points are right and I have pushed a fix for each. Two ports could be the same. This is a real bug and I missed it. Fixed the way you suggested, with one call that holds both sockets bound while it reads both def _get_address_and_ports() -> Tuple[str, int, int]:
"""Returns this node's address and two distinct free ports.
Both sockets stay bound until both port numbers have been read, so the
ports cannot come back equal. Two ``get_address_and_port`` calls can, since
each one releases its port before the next one probes.
"""
addr = ray.util.get_node_ip_address()
family = socket.AF_INET6 if is_ipv6(addr) else socket.AF_INET
with closing(socket.socket(family, socket.SOCK_STREAM)) as master_sock:
with closing(socket.socket(family, socket.SOCK_STREAM)) as comm_sock:
master_sock.bind(("", 0))
comm_sock.bind(("", 0))
return addr, master_sock.getsockname()[1], comm_sock.getsockname()[1]Three small deviations from the sketch, all so this matches what is already in the tree:
I looked for an existing "give me N free ports" helper to use instead and there is not one Implicit capture in the closure. Agreed, changed. def set_env_vars(addr, port, root_comm_port, socket_ifname):
...
worker_group.execute(
set_env_vars,
addr=master_addr,
port=master_port,
root_comm_port=root_comm_port,
socket_ifname=backend_config.socket_ifname,
)
One thing I want to keep flagged, since it has not changed: the exact diff here still has |
47371b7 to
e8bd0a2
Compare
There was a problem hiding this comment.
Code Review
This pull request updates the Torch XLA configuration to support multi-instance Neuron training by introducing a socket_ifname parameter and allocating two distinct free ports to prevent port collisions between the Neuron runtime rendezvous and torch.distributed. It also configures relevant environment variables like NEURON_RT_ROOT_COMM_ID, NCCL_SOCKET_IFNAME, and CCOM_SOCKET_IFNAME. The feedback suggests using "::" instead of "" as the bind address when the socket family is IPv6 (socket.AF_INET6) to ensure cross-platform compatibility and robust IPv6 support.
| family = socket.AF_INET6 if is_ipv6(addr) else socket.AF_INET | ||
| with closing(socket.socket(family, socket.SOCK_STREAM)) as master_sock: | ||
| with closing(socket.socket(family, socket.SOCK_STREAM)) as comm_sock: | ||
| master_sock.bind(("", 0)) | ||
| comm_sock.bind(("", 0)) | ||
| return addr, master_sock.getsockname()[1], comm_sock.getsockname()[1] |
There was a problem hiding this comment.
When family is socket.AF_INET6, binding to "" can fail with OSError on certain platforms (such as macOS or Windows) because "" is not always recognized as a valid wildcard address for IPv6. To ensure cross-platform compatibility and robust IPv6 support, use "::" as the bind address when family is socket.AF_INET6.
| family = socket.AF_INET6 if is_ipv6(addr) else socket.AF_INET | |
| with closing(socket.socket(family, socket.SOCK_STREAM)) as master_sock: | |
| with closing(socket.socket(family, socket.SOCK_STREAM)) as comm_sock: | |
| master_sock.bind(("", 0)) | |
| comm_sock.bind(("", 0)) | |
| return addr, master_sock.getsockname()[1], comm_sock.getsockname()[1] | |
| family = socket.AF_INET6 if is_ipv6(addr) else socket.AF_INET | |
| bind_addr = "::" if family == socket.AF_INET6 else "" | |
| with closing(socket.socket(family, socket.SOCK_STREAM)) as master_sock: | |
| with closing(socket.socket(family, socket.SOCK_STREAM)) as comm_sock: | |
| master_sock.bind((bind_addr, 0)) | |
| comm_sock.bind((bind_addr, 0)) | |
| return addr, master_sock.getsockname()[1], comm_sock.getsockname()[1] |
|
Thanks, but I would rather leave this one as it is, and I think the suggestion belongs
def find_free_port(family: socket.AddressFamily = socket.AF_INET) -> int:
with closing(socket.socket(family, socket.SOCK_STREAM)) as s:
s.bind(("", 0))
return s.getsockname()[1]
On the portability claim itself: CPython passes If you do consider |

Description
_TorchAwsNeuronXLABackendcannot bring up a Neuron collective that spans instances. Ontwo
trn1.32xlargehosts with 64 workers at one NeuronCore each, backend startup failsinside
_setup_xla_torch_process_groupwithThe Neuron runtime builds a cross-instance communicator by having every rank meet at the
endpoint named in
NEURON_RT_ROOT_COMM_ID, and nothing in Ray sets it.on_startalreadyasks worker 0 for
get_address_and_portand uses the answer forMASTER_ADDRandMASTER_PORT, so the address is already in hand; it is just never exported under the namethe runtime reads. A single instance has nothing to rendezvous with, which is why this is
invisible until the job crosses a host boundary.
This patch takes a second free port from worker 0 and sets
NEURON_RT_ROOT_COMM_ID=<worker 0 address>:<port>in the existingset_env_varsclosure,so every worker gets the same string. A second
get_address_and_portcall rather thanMASTER_PORTplus an offset, because the runtime needs an endpoint of its own andget_address_and_portreturns a port it has just verified free.Setting that alone was not sufficient in my environment, which is the part I did not
expect. With the root comm id correct on every rank, the run then died in the bootstrap:
CCOM selects its bootstrap socket by enumerating local interfaces and looking for one whose
subnet contains the peer address. Under the AWS VPC CNI each pod holds a /32, so no local
interface is ever in the same subnet as a peer and the search returns nothing. Naming the
interface skips the search. I want to be clear that this half is deployment specific: the
/32 comes from the CNI, and on bare EC2 instances where the host address sits in a real
subnet I would expect the match to succeed and this failure never to appear. I have not
tested that and cannot confirm it. The
NEURON_RT_ROOT_COMM_IDhalf is not conditional onany of that.
So the interface is an opt-in
socket_ifnamefield onTorchXLAConfigdefaulting toNone, not a hardcodedeth0.eth0is correct for a VPC CNI pod and wrong in general,and #42808 already removed
DEFAULT_NCCL_SOCKET_IFNAMEfrom Ray Train because users preferthese left alone; defaulting to
Nonekeeps that promise while making the knobdiscoverable next to
neuron_parallel_compile. When the field is set the backend usesos.environ.setdefault, so a value the user already exported wins, matching theTORCH_NCCL_ASYNC_ERROR_HANDLINGguard inpython/ray/train/torch/config.py.Open question for the reviewer, since I can see the argument both ways. The two interface
variables can already be set through
runtime_envenv_vars, so the field buysdiscoverability rather than capability, and you may prefer to drop it and document the
runtime_envroute instead.NEURON_RT_ROOT_COMM_IDgenuinely cannot be set that way,because its correct value is not known until the worker group exists. Happy to cut the
field if you would rather keep the config surface small.
NEURON_RT_ROOT_COMM_IDitself is set unconditionally, matching howMASTER_ADDRandMASTER_PORTare handled two lines above. If you would rather it also yield to apre-existing value, say so and I will change it.
Related issues
Closes #65070
Additional information
Tests:
trn1.32xlarge(32 NeuronCores each, 64 Ray Train workers at one coreper worker) on Kubernetes with the AWS VPC CNI and EFA enabled, running ray 2.56.1,
torch-neuronx 2.8.0.2.10.16998, torch-xla 2.8.1, neuronx-cc 2.21.33363.0, Python 3.11.11.
Before the change, backend startup fails with
Nrt::BuildGlobalComm failed. After it, theprocess group forms and a 64 rank job trains across both hosts: 24 steady state steps, no
NaN, and throughput around 90 percent of twice the single instance rate measured on the
same two hosts in the same session.
TorchXLAConfigsubclass overridingon_startbefore being written as a patch, and that is what the run above used. Thein-tree version differs only in taking the interface name from the config instead of
hardcoding it.
blackandruffclean on the changed file, at the repo's 88 column line length.that matters here is not observable without Trainium hardware: the env vars are trivially
assertable with a mocked worker group, but that only tests that the strings are spelled
correctly. I did not find a Trainium multi-instance runner in Ray's CI, so this path
cannot be covered there. If you want a mock-based unit test asserting the env vars land on
every worker, say the word and I will add one.
NEURON_RT_ROOT_COMM_IDis set butunused when there is nothing to rendezvous with, and
socket_ifnamedefaults toNonesono interface variable is touched unless asked for.