[distributed] ProcessGroupXCCL does not implement the c10d reconfigure API, so test_c10d_fault_tolerance.py has zero XPU coverage
- Dominant language
- Python
- Stars
- 113
- Forks
- 128
- Avg merge
- 5d 13h
- Merged PRs (30d)
- 107
Description
### Describe the bug
`ProcessGroupXCCL` does not implement the c10d fault-tolerance / reconfigure API, so it
inherits `return false` from `Backend.hpp:232`:
```cpp
// supportsReconfigure advertises support; get_reconfigure_handle returns an
// opaque handle that peers exchange out-of-band; reconfigure (re)initializes
// the communicator with a new set of peers.
virtual bool supportsReconfigure() const {
return false;
}
```
Only `ProcessGroupGloo.hpp:278` and `nccl2/ProcessGroupNCCL.hpp:302` override it.
`grep -rniE "reconfigure" src/xccl/` in this repo returns nothing.
Worse, `enable_reconfigure=True` is **silently accepted** on XPU -- no error, no warning,
the capability simply reports false:
```bash
python - <<'EOF'
import torch, torch.distributed as dist, tempfile
f = tempfile.NamedTemporaryFile(delete=False)
dist.init_process_group("xccl", world_size=1, rank=0,
store=dist.FileStore(f.name, 1), enable_reconfigure=True)
pg = dist.distributed_c10d._get_default_group()
b = dist.get_backend_impl(pg, torch.device("xpu"))
print("backend:", type(b).__name__)
print("dist._supports_reconfigure():", dist._supports_reconfigure())
print("backend.supports_reconfigure:", b.supports_reconfigure)
EOF
```
```
backend: ProcessGroupXCCL
dist._supports_reconfigure(): False
backend.supports_reconfigure: False
```
Arguably a smaller bug in its own right: asking for `enable_reconfigure=True` on a backend
that cannot honour it should raise (or at least warn) rather than hand back a process group
that quietly lacks the capability.
### What XPU needs
1. `supportsReconfigure()` returning `true`, plus `getReconfigureHandle()` and
`reconfigure()` -- the handle is an opaque blob peers exchange out-of-band, and
`reconfigure()` must re-initialize the communicator against a new peer set (including
shrinking, merging, and recovering after `abort()`).
2. `get_error()` reporting `ErrorType::COMM_ERROR` after an `abort()` and
`ErrorType::TIMEOUT` after a collective times out (the tests assert both).
3. Work-result reporting (`Work::get_future_result()` -> `WorkResult::TIMEOUT` /
`COMM_ERROR`), which is what `FaultToleranceBackend.supports_work_result` gates.
### Test impact
`test/distributed/test_c10d_fault_tolerance.py` builds one test class per entry in
```python
FAULT_TOLERANCE_BACKENDS = [
FaultToleranceBackend("gloo", "cpu"),
FaultToleranceBackend("nccl2", "cuda", supports_work_result=True),
]
```
so on XPU (`python test/distributed/test_c10d_fault_tolerance.py -v`) the picture is:
```
Ran 34 tests in 42.110s
OK (skipped=19)
```
| Class | Collected | Pass | Skip |
|---|---|---|---|
| `GlooFaultToleranceTest` (cpu) | 16 | 13 | 3 (2x "gloo does not report work results", 1x nonblocking-NCCL-init) |
| `Nccl2FaultToleranceTest` (cuda) | 16 | 0 | 16 -- "fault tolerance CUDA tests require at least 3 GPUs" (`not TEST_CUDA`, line 392) |
| `ReconfigureContractTest` / `BackendCapabilityContractTest` | 2 | 2 | 0 (device-free) |
So all 16 accelerator-path tests are dead on XPU, and the only thing exercising reconfigure
is the CPU/gloo class.
Note this file is *less* forgiving than `test_c10d_window.py` (see #5407): it has no
capability-based self-skip. Adding `FaultToleranceBackend("xccl", "xpu",
supports_work_result=True)` today would make all 16 tests **hard-fail**, not skip, at the
two assertions in `_init_reconfigurable_pg` (lines 81-82):
```python
self.assertTrue(dist._supports_reconfigure())
self.assertTrue(self.backend.supports_reconfigure)
```
### What must pass to close this
With `("xccl", "xpu")` added to `FAULT_TOLERANCE_BACKENDS`, the generated
`XcclFaultToleranceTest` class must run all 16 tests green with no skips:
```bash
python test/distributed/test_c10d_fault_tolerance.py -v
```
- `XcclFaultToleranceTest::test_reconfigure_basic`
- `XcclFaultToleranceTest::test_reconfigure_then_all_reduce`
- `XcclFaultToleranceTest::test_reconfigure_then_send_recv`
- `XcclFaultToleranceTest::test_reconfigure_identity`
- `XcclFaultToleranceTest::test_reconfigure_scale_down_up`
- `XcclFaultToleranceTest::test_reconfigure_single_to_all`
- `XcclFaultToleranceTest::test_reconfigure_late_join`
- `XcclFaultToleranceTest::test_reconfigure_merge_split`
- `XcclFaultToleranceTest::test_shrink_exclude_last_rank`
- `XcclFaultToleranceTest::test_shrink_exclude_middle_rank`
- `XcclFaultToleranceTest::test_reconfigure_after_abort`
- `XcclFaultToleranceTest::test_reconfigure_after_timeout`
- `XcclFaultToleranceTest::test_reconfigure_rejects_reused_uuid`
- `XcclFaultToleranceTest::test_reconfigure_timeout_is_retryable`
- `XcclFaultToleranceTest::test_work_explicit_timeout_includes_prelaunch_stall`
- `XcclFaultToleranceTest::test_work_reports_communicator_error`
The last two are the ones needing item 3 above; the class-level gate at lines 390-393
(`not TEST_CUDA or torch.cuda.device_count() < 3`) also has to become accelerator-generic,
and the suite needs 3 devices.
### Note: PyTorch-side prep work
Independently of XCCL, the test file itself has to be made device-generic before an XPU
entry can work. These are PyTorch changes (part of pytorch/pytorch#114850), listed so the
two halves are not fixed in isolation:
- `device` property hardcodes `f"cuda:{self.rank}"` (lines 50-51) and `_init_reconfigurable_pg`
calls `torch.cuda.set_device` (lines 72-73)
- `test_work_explicit_timeout_includes_prelaunch_stall` uses `torch.cuda.synchronize()`,
`torch.cuda._sleep()` and `torch.cuda.current_stream().query()` (lines 154-166)
- `test_work_reports_communicator_error` uses `torch.cuda.synchronize()` (line 173) and
asserts the literal backend string `"NCCL operation failed"` (line 186)
- backend-name branches at lines 293, 336 and 358
### Versions
```
torch 2.15.0a0+git3c3da2a (source build, commit 3c3da2a5e8b)
device 4x Intel(R) Data Center GPU Max 1100
backend xccl
```
Contributor guide
Research direction
Read Backend.hpp:232 and the XCCL sources under src/xccl/ to map the missing reconfigure, error, and work-result interfaces. Then run test/distributed/test_c10d_fault_tolerance.py -v and review the listed PyTorch-side device-generic changes and XPU test requirements. Done means an XcclFaultToleranceTest class runs all 16 tests green with no skips.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- distributed-systems, testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100