Skip to content

ci(hipblaslt): shorten ASAN CI test timeout and log the runner on hang - #12728

Merged
tony-davis merged 1 commit into
developfrom
users/todavis/hipblaslt-asan-ci-test-timeout
Sep 30, 2026
Merged

tony-davis merged 1 commit into
developfrom
users/todavis/hipblaslt-asan-ci-test-timeout

Conversation

@tony-davis

Copy link
Copy Markdown
Contributor

ISSUE ID : #12727

Motivation

The hipBLASLt ASAN CI workflow runs the hipBLASLt AddressSanitizer smoke tests on every
touching PR, and its test step is not currently a reliable read on the change under review.
When the test job hangs — which recent runs show happening far more often than an actual
test or sanitizer failure — the step keeps running for its full 180-minute timeout before
being killed, and nothing in the log says which runner it ran on. A silent three-hour wait
that looks identical to a real hang leaves the lane effectively unusable as a review signal.

Technical Details

  • Shortens the test job's overall timeout from 210 to 90 minutes, and the
    Test hipBLASLt (quick) step's timeout from 180 to 30 minutes.
  • Wraps the hipblaslt-test invocation in timeout -k 60 20m. When that fires (exit code
    124 or 137), the step prints an ::error:: line naming the runner and the 20-minute
    cutoff, then exits with that code; any other failure keeps its own original exit code.
  • Prints the runner name (${{ runner.name }}) at the start of the sanity-check step, so a
    future hang can be attributed to a specific host — GitHub's API does not otherwise expose
    which runner ran a job.
  • Configuration only: no product code, test content, or ASAN option changes.

Test Plan

  • Local dry run of the new step logic (hang / real failure / pass) against a stand-in
    script, and actionlint on the modified workflow.
  • The workflow triggers on this PR automatically, since it watches its own file — that run
    is this change's own test.

Test Result

  • actionlint: no findings.
  • Local simulation: a stand-in 300-second hang under timeout -k 60 20m was killed at the
    cap (exit 124), printed the expected ::error:: line, and left no child process running
    afterward. A stand-in non-zero exit passed its original code through unchanged. A stand-in
    pass exited 0.
  • Automated GitHub checks, including this PR's own hipBLASLt ASAN CI run, are pending.

Submission Checklist

Risk level

Low (2/5): narrow change to one non-required, informational CI lane, with no product code
or test content changed. The only effect on a passing run is the lower time cap — passing
smoke runs measured at 2-3 minutes, so the new 20-minute step cap and 90-minute job cap
leave a wide margin — and this lane isn't a required check, so an unexpected early cutoff
would not block a merge.

Related

🤖 Generated with Claude Code

The hipBLASLt ASAN CI test step hangs to its full 180-minute timeout on a
meaningful share of runs instead of failing on an actual test or sanitizer
result. Shorten the test step to a 20-minute cap (30-minute step limit,
90-minute job limit) so a hang fails fast with a clear error, and log the
runner name so a future hang can be attributed to a specific host.

ISSUE ID : #12727

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added github actions project: none Does not target any component labels Sep 29, 2026
@therock-pr-bot

therock-pr-bot Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

⏳ Waiting on Required Checks

⏳ Required checks have not all concluded yet. This bot run is exiting without failing so those checks can report independently.

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⏳ Required checks are still running or have not started yet.
This PR is not ready for review until they complete.

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

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

⏳ Waiting for required checks to start or finish:

  • ⏳ pre-commit

@tony-davis
tony-davis marked this pull request as ready for review September 29, 2026 19:26
Copilot AI balanced review requested due to automatic review settings September 29, 2026 19:26
@tony-davis
tony-davis requested a review from a team as a code owner September 29, 2026 19:26

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@tony-davis

Copy link
Copy Markdown
Contributor Author

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Boo!

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The only finding is a minor diagnostic wording nit; no blocking issues were identified.

Review effort: Lite
Findings: 1 Low severity

Open (1)

Comment thread .github/workflows/hipblaslt-asan-ci.yml Resolved
@tony-davis
tony-davis requested a review from talumbau September 30, 2026 17:43
@tony-davis
tony-davis merged commit 530a62b into develop Sep 30, 2026
481 of 510 checks passed
@tony-davis
tony-davis deleted the users/todavis/hipblaslt-asan-ci-test-timeout branch September 30, 2026 17:48
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Comment on lines +262 to +265
env:
RUNNER_NAME: ${{ runner.name }}
run: |
echo "Runner: ${RUNNER_NAME}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Prints the runner name (${{ runner.name }}) at the start of the sanity-check step, so a
    future hang can be attributed to a specific host — GitHub's API does not otherwise expose
    which runner ran a job.

FYI the second line of log output on every github actions workflow run is the runner name, e.g. https://github.com/ROCm/rocm-libraries/actions/runs/36607831817/job/109615347152#step:1:2

Current runner version: '2.336.0'
Runner name: 'smci250-ccs-aus-c05-13-dev-1gpu-gfx90a-gpu1-1790722069'
Runner group name: 'default'
Machine name: 'smci250-ccs-aus-c05-13-dev-1gpu-gfx90a-gpu1'
GITHUB_TOKEN Permissions
  Contents: read
  Metadata: read
Secret source: Actions
Cache mode: write

sa-faizal added a commit that referenced this pull request Oct 8, 2026
ISSUE ID: #12727

## Motivation
The hipBLASLt per-PR ASAN lane has been hanging under full ASAN with
`HSA_XNACK=1`. This switches it to host-only ASAN with XNACK off,
matching the coverage already described in `TESTING.md`.

GPU correctness tests still run, and ASAN still checks CPU memory
accesses. The trade-off is that no gfx90a device-ASAN runtime tests
remain in CI. Before restoring that coverage, we need to review runner
capacity and full-ASAN stability with DevOps, potentially for a
dedicated nightly pool. The cause of the hangs remains under
investigation.

[#12728](#12728) added
shorter timeouts and diagnostics.
[#12813](#12813) offers an
alternative runner-pool workaround. The investigation is tracked in
[TheRock-Infra #978](ROCm/TheRock-Infra#978).

## Technical Details
- Use `HOST_ASAN` artifacts and `HSA_XNACK=0` in the focused lane.
- Stop requiring `xnack+` targets for host-only ASAN; preserve existing
full-ASAN behavior.
- Update `TESTING.md` and remove the stale gfx942 `ci:asan` row.

## Test Plan
Run the existing gfx90a build and smoke tests, plus whitespace and
applicable pre-commit checks.

## Test Result
[Build and all 950 smoke tests
passed](https://github.com/ROCm/rocm-libraries/actions/runs/37402899925)
on `c92deaa`. The documentation follow-up passes whitespace checks;
pre-commit completed with no applicable hooks. CI for the latest commit
must pass before merge.

## Submission Checklist
- [x] Reviewed the [contributing
guidelines](https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests).

## Risk level
Medium (3/5): retains host-ASAN and GPU correctness checks, but removes
device-ASAN coverage from the gfx90a lane. A passing run does not prove
the hangs are fixed.

---------

Co-authored-by: Sahil Faizal <197828758+sa-faizal@users.noreply.github.com>
Co-authored-by: Sahil Faizal <safaizal@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants