Skip to content

test(rccl): cover GIN-success then RMA-fail obtain rollback (AICOMRCCL-2428) - #12705

Merged
dlamd1dai merged 2 commits into
developfrom
users/dondai/aicomrccl-2428-gin-rma-rollback-test
Oct 8, 2026
Merged

dlamd1dai merged 2 commits into
developfrom
users/dondai/aicomrccl-2428-gin-rma-rollback-test

Conversation

@dlamd1dai

Copy link
Copy Markdown
Contributor

Motivation

Follow-up to #11863 / AICOMRCCL-2241, which has merged. Argus noted that free(mem->ginSegmentInfos) on the fail_mem path is never exercised: reaching it needs GIN register to succeed and RMA register to then fail. Existing host coverage either fails GIN first (GinRegisterFails_UnbindsTeamsAndReturnsSpace) or fails RMA with ginEnabled false (RmaRegisterFails_ReturnsSpaceWithoutLinking).

Supersedes #12082, which was opened from a fork branch and so could not pass the RCCL PR gate coco smoke check. Same single commit, same content, now on a branch in this repo.

Technical Details

Adds SymMemoryObtainRollbackTest.GinSucceedsThenRmaFails_UnregistersGinAndUnlinks in rccl-UnitTestsMicro (test/host/dev-runtime-test.cc):

  • ginEnabled true, GIN register succeeds, RMA register fails
  • Asserts symMemoryUnregister's GIN arm ran (ncclGinDeregister once)
  • Asserts RMA deregister is skipped (rmaHostWins[0] never set)
  • Asserts memHead is unlinked and reserved space is returned

free(ginSegmentInfos) sits on the same fail path after unregister; the deregister call is the observable witness because mem is freed before Obtain returns.

Remaining AICOMRCCL-2428 items (shared unlink helper, rccl-UnitTestsDevRuntime CI wiring, real-TU null-skip deregister coverage) stay for later PRs.

JIRA ID: AICOMRCCL-2428

Test Plan

rccl-UnitTestsMicro --gtest_filter=SymMemoryObtainRollbackTest.GinSucceedsThenRmaFails_UnregistersGinAndUnlinks

Host CPU-only CI (RCCL host unit tests) also runs the suite via rccl-UnitTestsMicro.

Test Result

Host unit tests (CPU-only) passed on this commit in #12082 (run 37050708055): all five SymMemoryObtainRollbackTest cases ran green, including the new one.

[ RUN      ] SymMemoryObtainRollbackTest.GinSucceedsThenRmaFails_UnregistersGinAndUnlinks
[       OK ] SymMemoryObtainRollbackTest.GinSucceedsThenRmaFails_UnregistersGinAndUnlinks (0 ms)

Submission Checklist

…L-2428)

Exercise fail_mem_space_teams after a successful GIN register so
symMemoryUnregister's GIN arm and free(ginSegmentInfos) run, then assert
memHead is unlinked. Kept out of #11863 so AICOMRCCL-2241 can land first.
@therock-pr-bot

therock-pr-bot Bot commented Oct 2, 2026

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rccl/test/host/dev-runtime-test.cc; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@amd-argus amd-argus Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARGUS

Bottom line: LGTM.

Comment @argus review to re-run, or @argus re-review after pushing.

Name the fail_mem path and ginDereg witness the way the review asked.

Co-authored-by: Cursor <cursoragent@cursor.com>

@amd-argus amd-argus Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARGUS

Bottom line: LGTM.

Comment @argus review to re-run, or @argus re-review after pushing.

@dlamd1dai

Copy link
Copy Markdown
Contributor Author

CI analysis: the Ruby RCCL PR gate failure is a cluster-node problem, not this PR

Four checks are non-green here. Only RCCL PR gate is required on develop, and it is red solely because of the Ruby leg. The checks that actually exercise this change — Host unit tests (CPU-only), mci/rocm-libraries/precheckin(rccl), extra(rccl), static-analysis(rccl) — all pass.

1. Ruby / ruby64-rccl-pr-smoke — fails identically on develop

coco ran both stacks and reported:

INFO [pr-12705_rccl-debug_rocm-10.1-latest] passed=201, skipped=6
INFO [pr-12705_rccl-perf_rocm-10.1-latest]  passed=17, failed=5, skipped=3
INFO [rocm-10.1-latest_rccl-develop-perf]   passed=17, failed=5, skipped=3

The third line is the develop baseline, which does not contain this change. It fails the same five tests by name, not just the same count:

Test Topology
regression-allgather-fp32_all_gather_perf 4n_8ppn
regression-alltoall-fp32_alltoall_perf 4n_8ppn
regression-bf16-sum_all_reduce_perf 4n_8ppn
regression-bf16-sum_reduce_scatter_perf 4n_8ppn
regression-reduce-scatter-fp32-sum_reduce_scatter_perf 4n_8ppn

Every failure is 4-node. All 1-node and 2-node variants pass, the debug stack passes 201/207, and OCI passed all five of these same 4-node tests.

In all ten failures (five tests × two stacks) the same node reports the primary error and the others report a cascade from it:

cv350-rck-g03-e14-08: Test NCCL failure ... 'unhandled system error (run with NCCL_DEBUG=INFO for details)'
cv350-rck-g03-c17-08: Test NCCL failure ... 'remote process exited or there was a network error'
cv350-rck-g03-e20-08: Test NCCL failure ... 'remote process exited or there was a network error'

The Slurm allocations explain the topology pattern:

Topology Nodes Result
1n_8ppn c17-08 pass
2n_8ppn e06-08, e06-18 pass
4n_8ppn c17-08, c17-18, e14-08, e20-08 fail

c17-08 appears in both the passing 1-node run and the failing 4-node run, so that node is healthy. What changes is the presence of e14-08.

2. Cross-PR confirmation

#12730 shows the identical failure one day earlier with a different 4-node allocation (c07-18, e13-08, e14-08, e20-08) — same five tests, same primary node cv350-rck-g03-e14-08, same baseline parity, OCI green. Two unrelated PRs, two days, two allocations, one common node. That rules out both changes as the cause.

This PR additionally can't be implicated: it adds a single host-side gtest, while the failing tests are rccl_test perf regressions launched through mpirun.

Requests

  1. Can someone confirm whether cv350-rck-g03-e14-08 is known-bad, and drain or repair it? Proper diagnosis needs NCCL_DEBUG=INFO, which the log asks for and the perf harness doesn't set.
  2. If it can't be fixed quickly, this PR needs a gate override on the baseline-parity evidence above.
  3. Structural suggestion: coco already collects the baseline stack and prints its results beside the PR's, but nothing compares them — the gate fails on any test failure rather than on new failures. Diffing the two and failing only on regressions would let PRs like this and rccl-tests: share GIN-SDMA pytest harness #12730 pass without human intervention when a cluster node is down.

3. build (RCCL Perf Regression Gate) — stuck queued, and blocking other PRs

This check has been queued since 06:50 UTC waiting on runs-on: [self-hosted, tw-gfx950-ainic-ROCm-scale-runner]. Across the last 100 runs of that workflow, zero completed: 88 cancelled, 9 skipped, 3 stuck. The runner does not appear to be picking up jobs.

There is also a concurrency issue in .github/workflows/rccl_perf_regression.yml:

concurrency:
  group: rccl-perf-gate-build
  cancel-in-progress: false

The group is a constant with no github.ref, so every RCCL PR contends for a single global lock. GitHub keeps only one pending run per group and cancels the rest, which accounts for the 88 cancellations. This PR's run currently holds that lock while waiting for an unavailable runner, starving the RCCL PRs behind it. Adding github.ref to the group key would stop PRs from cancelling each other.

This check is not required on develop, so it does not block this PR.

4. therock-pr-bot — systemic, non-blocking

"Timed out waiting for required checks to complete": the bot waits for a pre-commit check-run that only triggers on testbranch, so it never appears on PRs targeting develop. Its own policy comment on this PR reports All Policy Checks Passed. Its unit-test warning is a false positive — it flagged "code file changed without accompanying unit test" when the changed file is the unit test.

@dlamd1dai
dlamd1dai merged commit a9d3abd into develop Oct 8, 2026
95 of 102 checks passed
@dlamd1dai
dlamd1dai deleted the users/dondai/aicomrccl-2428-gin-rma-rollback-test branch October 8, 2026 19:41
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

anujshuk-amd pushed a commit that referenced this pull request Oct 12, 2026
…L-2428) (#12705)

## Motivation

Follow-up to #11863 / AICOMRCCL-2241, which has merged. Argus noted that
`free(mem->ginSegmentInfos)` on the `fail_mem` path is never exercised:
reaching it needs GIN register to succeed and RMA register to then fail.
Existing host coverage either fails GIN first
(`GinRegisterFails_UnbindsTeamsAndReturnsSpace`) or fails RMA with
`ginEnabled` false (`RmaRegisterFails_ReturnsSpaceWithoutLinking`).

Supersedes #12082, which was opened from a fork branch and so could not
pass the `RCCL PR gate` coco smoke check. Same single commit, same
content, now on a branch in this repo.

## Technical Details

Adds
`SymMemoryObtainRollbackTest.GinSucceedsThenRmaFails_UnregistersGinAndUnlinks`
in `rccl-UnitTestsMicro` (`test/host/dev-runtime-test.cc`):

- `ginEnabled` true, GIN register succeeds, RMA register fails
- Asserts `symMemoryUnregister`'s GIN arm ran (`ncclGinDeregister` once)
- Asserts RMA deregister is skipped (`rmaHostWins[0]` never set)
- Asserts `memHead` is unlinked and reserved space is returned

`free(ginSegmentInfos)` sits on the same fail path after unregister; the
deregister call is the observable witness because `mem` is freed before
`Obtain` returns.

Remaining AICOMRCCL-2428 items (shared unlink helper,
`rccl-UnitTestsDevRuntime` CI wiring, real-TU null-skip deregister
coverage) stay for later PRs.

JIRA ID: AICOMRCCL-2428

## Test Plan

```
rccl-UnitTestsMicro --gtest_filter=SymMemoryObtainRollbackTest.GinSucceedsThenRmaFails_UnregistersGinAndUnlinks
```

Host CPU-only CI (`RCCL host unit tests`) also runs the suite via
`rccl-UnitTestsMicro`.

## Test Result

`Host unit tests (CPU-only)` passed on this commit in #12082 (run
37050708055): all five `SymMemoryObtainRollbackTest` cases ran green,
including the new one.

```
[ RUN      ] SymMemoryObtainRollbackTest.GinSucceedsThenRmaFails_UnregistersGinAndUnlinks
[       OK ] SymMemoryObtainRollbackTest.GinSucceedsThenRmaFails_UnregistersGinAndUnlinks (0 ms)
```

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants