Repository navigation
Conversation
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Contributor
Author
|
/ok to test 85b82ce |
Contributor
|
Light review - LGTM Clean cleanup. Replacing the socket-based init_method with an in-process dist.HashStore() is the right call for a single-process (world_size=1, rank=0) context: it avoids reserving/leaking a TCP port and no longer inherits rendezvous env vars from the caller. Verified the removed os/socket imports and the deleted test_temporary_distributed_context_with_env_vars test (the env-var branch it covered no longer exists), and confirmed no remaining doc/script references to the old behavior. Call-site trace confirms consistency:
No critical bugs or typos found. Note: --standalone alongside --nnodes=1 is redundant but harmless. Suggested test cases
|
yaoyu-33
added
area:training
ManoharPaturi
added a commit
to ManoharPaturi/Megatron-Bridge
that referenced
this pull request
Sep 7, 2026
…zvous temporary_distributed_context() picked a free TCP port, closed the socket, and then asked init_process_group() to bind the same address. Any process claiming the port in that window makes standalone checkpoint import/export fail with EADDRINUSE (race seen in public CI). When MASTER_ADDR/MASTER_PORT were set the context also silently inherited the caller's rendezvous settings. The context is always world_size=1 rank=0, so rendezvous over TCP is not needed at all: pass an in-process dist.HashStore() instead. No host port is touched and env vars are ignored. Fixes NVIDIA-NeMo#5967. Approach follows yaoyu-33's NVIDIA-NeMo#5595 (closed before review finished). Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
torch.distributed.HashStorefor singleton temporary process groups, avoiding TCP rendezvous ports and inherited distributed environment settingstorchrun --standalonefor the Qwen3 adapter TP/PP verification subprocessesRoot cause
Main CI job https://github.com/NVIDIA-NeMo/Megatron-Bridge/actions/runs/31846107309/job/94940794151 failed in
TestVerifyAdapter.test_cpu_full_verificationwithEADDRINUSEon port 43963.The test selected a free port in the parent process, released it, and then launched a child that spent about 20 seconds loading models before binding that port. Another socket could claim the port during that gap.
Testing
uvx --from pre-commit pre-commit run --all-files— passedgit diff --check— passedHashStorewhileMASTER_PORTpointed to an occupied socket — passeduv run python -m pytest tests/unit_tests/training/test_model_load_save.py -k temporary_distributed_context -vv— not runnable on macOS ARM becausenvidia-resiliency-ext==0.6.0publishes Linux-only wheels; CI will run the focused coverage on Linux