Skip to content

Improve compute sanitizer workflow #360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bdice wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Improve compute sanitizer workflow #360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95c777b
Update compute sanitizer CUDA image default
bdice Jun 12, 2026
0d83064
Align compute sanitizer workflow split
bdice Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .github/workflows/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ This directory contains GitHub Actions workflows for automated testing, benchmar
| `presto-test.yml` | Reusable workflow for Presto CI image tests | Smoke + integration tests; supports `workflow_dispatch` for test-only runs |
| `ci-image-cleanup.yml` | Deletes CI images older than 30 days from GHCR | Weekly (Tuesdays) + manual dispatch |
| **Compute Sanitizer** |||
| `velox-compute-sanitizer-trigger.yaml` | Discovers cuDF tests and runs compute-sanitizer tools | Weekly (Saturdays) + manual dispatch |
| `velox-compute-sanitizer-run.yaml` | Reusable workflow to run a sanitizer tool on a matrix of tests | Called by trigger; also supports `workflow_dispatch` |
| `velox-compute-sanitizer-trigger.yaml` | Triggers compute-sanitizer tool workflows | Weekly (Saturdays) + manual dispatch |
| `velox-compute-sanitizer-run.yaml` | Discovers cuDF tests and runs one sanitizer tool over the matrix | Triggered by schedule workflow; supports targeted manual dispatch |

---

Expand Down Expand Up @@ -231,8 +231,9 @@ presto-test.yml (workflow_dispatch) ──► [test images by SHA/date/variant]

COMPUTE SANITIZER
─────────────────
velox-compute-sanitizer-trigger.yaml ──► [discover cuda_driver tests] ──► velox-compute-sanitizer-run.yaml (racecheck)
└─► velox-compute-sanitizer-run.yaml (synccheck)
velox-compute-sanitizer-trigger.yaml ──► velox-compute-sanitizer-run.yaml (memcheck) ──► [discover cuda_driver tests]
├─► velox-compute-sanitizer-run.yaml (racecheck) ──► [discover cuda_driver tests]
└─► velox-compute-sanitizer-run.yaml (synccheck) ──► [discover cuda_driver tests]

CLEANUP
───────
Expand Down
85 changes: 63 additions & 22 deletions .github/workflows/velox-compute-sanitizer-run.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,7 @@
name: compute-sanitizer-run
name: Compute Sanitizer
run-name: "Compute Sanitizer ${{ inputs.tool_name }}"

on:
workflow_call:
inputs:
tool_name:
required: true
type: string
description: "Compute sanitizer tool to run (memcheck, racecheck, initcheck, synccheck)"
test_names:
required: true
type: string
description: "JSON array of test names to run"
velox_image:
required: true
type: string
description: "Full Velox GPU container image reference"
workflow_dispatch:
inputs:
tool_name:
Expand All @@ -27,20 +14,74 @@ on:
- initcheck
- synccheck
test_names:
required: true
required: false
type: string
description: "JSON array of test names to run"
description: "JSON array of test names to run (discovers all tests if empty)"
default: ""
velox_image:
required: true
required: false
type: string
description: "Full Velox GPU container image reference"
description: "Velox GPU image to test (e.g. ghcr.io/rapidsai/velox-testing-images:velox-e0169a0-gpu-cuda13.1-20260318). Leave empty to use velox-latest-gpu-cuda13.1."
default: ""

permissions: {}

env:
REGISTRY: ghcr.io
IMAGE_NAME: ${{ github.repository }}-images

jobs:
resolve-image:
runs-on: ubuntu-latest
outputs:
velox_image: ${{ steps.resolve-image.outputs.velox_image }}
steps:
- name: Resolve image
id: resolve-image
env:
VELOX_IMAGE: ${{ inputs.velox_image }}
run: |
if [ -n "${VELOX_IMAGE}" ]; then
echo "velox_image=${VELOX_IMAGE}" >> "${GITHUB_OUTPUT}"
else
echo "velox_image=${REGISTRY}/${IMAGE_NAME}:velox-latest-gpu-cuda13.1" >> "${GITHUB_OUTPUT}"
fi

discover-sanitizer-tests:
if: ${{ !inputs.test_names }}
needs: resolve-image
runs-on: linux-amd64-cpu4
permissions:
contents: read
packages: read
outputs:
tests: ${{ steps.find-tests.outputs.tests }}
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false
- name: Log in to the Container registry
uses: ./.github/actions/container-registry-login
- name: Discover test executables
id: find-tests
env:
IMAGE: ${{ needs.resolve-image.outputs.velox_image }}
run: |
source scripts/common.sh

docker_pull_with_retry "${IMAGE}"
docker run --rm \
-v "${GITHUB_WORKSPACE}/ci/discover_velox_cudf_tests.sh:/tmp/discover.sh:ro" \
-e GITHUB_OUTPUT=/tmp/gh_output \
"${IMAGE}" bash -c "bash /tmp/discover.sh && cat /tmp/gh_output" >> "${GITHUB_OUTPUT}"

run-sanitizer-tests:
name: ${{ inputs.tool_name }} / ${{ matrix.test_name }}
name: Run ${{ inputs.tool_name }} on ${{ matrix.test_name }}
needs:
- resolve-image
- discover-sanitizer-tests
if: ${{ !cancelled() }}

@misiugodfrey misiugodfrey Jun 25, 2026

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.

Does !cancelled() override the default behavior of "skip if dependent job failed"?

My understanding of the intent here is that we want to run this even if discover-sanitizer-tests was skipped (i.e. if tests to run were manually specified), but I don't think we want to run if it, or the resolve-image steps failed.

Perhaps something like: if: ${{ !cancelled() && (inputs.test_names != '' }} conveys the intend more explicitly?

@bdice bdice Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will check into this.

runs-on: linux-amd64-gpu-l4-latest-1
permissions:
contents: read
Expand All @@ -49,7 +90,7 @@ jobs:
strategy:
fail-fast: false
matrix:
test_name: ${{ fromJson(inputs.test_names) }}
test_name: ${{ fromJson(inputs.test_names || needs.discover-sanitizer-tests.outputs.tests) }}
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
Expand All @@ -59,7 +100,7 @@ jobs:
uses: ./.github/actions/container-registry-login
- name: Run compute-sanitizer ${{ inputs.tool_name }} on ${{ matrix.test_name }}
env:
IMAGE: ${{ inputs.velox_image }}
IMAGE: ${{ needs.resolve-image.outputs.velox_image }}
TOOL_NAME: ${{ inputs.tool_name }}
TEST_NAME: ${{ matrix.test_name }}
run: |
Expand Down
99 changes: 25 additions & 74 deletions .github/workflows/velox-compute-sanitizer-trigger.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,97 +1,48 @@
name: compute-sanitizer-trigger
name: Trigger Compute Sanitizer

# Runs compute-sanitizer tools on all Velox cuDF (cuda_driver) tests.
# This workflow triggers compute-sanitizer runs for all Velox cuDF (cuda_driver) tests weekly.
# WARNING: This is resource intensive and runs many GPU jobs.
# For targeted testing, manually trigger compute-sanitizer-run.yaml with specific inputs.
# For targeted testing, manually trigger velox-compute-sanitizer-run.yaml with specific tool_name and test_names.

on:
schedule:
- cron: '0 10 * * 6' # Weekly on Saturday at 10:00 UTC
- cron: "0 10 * * 6" # Weekly on Saturday at 10:00 UTC
workflow_dispatch:
inputs:
velox_image:
description: 'Velox GPU image to test (e.g. ghcr.io/rapidsai/velox-testing-images:velox-e0169a0-gpu-cuda13.1-20260318). Leave empty to use velox-latest-gpu-cuda12.9.'
description: "Velox GPU image to test (e.g. ghcr.io/rapidsai/velox-testing-images:velox-e0169a0-gpu-cuda13.1-20260318). Leave empty to use velox-latest-gpu-cuda13.1."
type: string
required: false
default: ''
default: ""

@kingcrimsontianyu kingcrimsontianyu Jun 25, 2026

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.

Is there any difference between using single/double quotes in this yaml file? Or is this simply to maintain format consistency and use double quotes wherever possible?


permissions: {}

env:
REGISTRY: ghcr.io
IMAGE_NAME: ${{ github.repository }}-images

jobs:
discover-tests:
runs-on: linux-amd64-cpu4
trigger-sanitizer:
name: Trigger ${{ matrix.tool_name }}
runs-on: ubuntu-latest
permissions:
actions: write
contents: read
packages: read
outputs:
tests: ${{ steps.find-tests.outputs.tests }}
velox_image: ${{ steps.resolve-image.outputs.velox_image }}
strategy:
fail-fast: false
matrix:
tool_name:
- memcheck
- racecheck
- synccheck
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

@misiugodfrey misiugodfrey Jun 25, 2026

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.

I don't think trigger-sanitizer needs the checkout action any more? Since it's just calling gh workflow run now, I think we can remove this action (and possibly the contents: read requirement).

@bdice bdice Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably so. I will check that.

@kingcrimsontianyu kingcrimsontianyu Jun 25, 2026

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.

I'm not familiar with workflow management with yaml in general. Claude flagged a minor issue. Not sure if it makes sense or not. Just fyi:

velox-compute-sanitizer-trigger.yaml:35 — the actions/checkout step is unnecessary. The job's only real step is gh workflow run …, which is a pure REST call (uses GH_TOKEN + the auto-set GITHUB_REPOSITORY); it never reads the working tree. The checkout — and the contents: read permission it forces at line 26 — is dead work that can be removed, leaving the job with just actions: write.

with:
persist-credentials: false
- name: Log in to the Container registry
uses: ./.github/actions/container-registry-login
- name: Resolve image
id: resolve-image
- name: Trigger ${{ matrix.tool_name }}
env:
VELOX_IMAGE: ${{ inputs.velox_image }}
GH_TOKEN: ${{ github.token }}
TOOL_NAME: ${{ matrix.tool_name }}
VELOX_IMAGE: ${{ github.event.inputs.velox_image }}
run: |
args=(--ref "${GITHUB_REF_NAME}" -f tool_name="${TOOL_NAME}")
if [ -n "${VELOX_IMAGE}" ]; then
echo "velox_image=${VELOX_IMAGE}" >> "${GITHUB_OUTPUT}"
else
echo "velox_image=${REGISTRY}/${IMAGE_NAME}:velox-latest-gpu-cuda12.9" >> "${GITHUB_OUTPUT}"
args+=(-f velox_image="${VELOX_IMAGE}")
fi
- name: Discover test executables
id: find-tests
env:
IMAGE: ${{ steps.resolve-image.outputs.velox_image }}
run: |
source scripts/common.sh

docker_pull_with_retry "${IMAGE}"
docker run --rm \
-v "${GITHUB_WORKSPACE}/ci/discover_velox_cudf_tests.sh:/tmp/discover.sh:ro" \
-e GITHUB_OUTPUT=/tmp/gh_output \
"${IMAGE}" bash -c "bash /tmp/discover.sh && cat /tmp/gh_output" >> "${GITHUB_OUTPUT}"

run-sanitizer-tests-memcheck:
name: compute-sanitizer memcheck
needs: discover-tests
permissions:
contents: read
packages: read
uses: ./.github/workflows/velox-compute-sanitizer-run.yaml
with:
tool_name: "memcheck"
test_names: ${{ needs.discover-tests.outputs.tests }}
velox_image: ${{ needs.discover-tests.outputs.velox_image }}

run-sanitizer-tests-racecheck:
name: compute-sanitizer racecheck
needs: discover-tests
permissions:
contents: read
packages: read
uses: ./.github/workflows/velox-compute-sanitizer-run.yaml
with:
tool_name: "racecheck"
test_names: ${{ needs.discover-tests.outputs.tests }}
velox_image: ${{ needs.discover-tests.outputs.velox_image }}

run-sanitizer-tests-synccheck:
name: compute-sanitizer synccheck
needs: discover-tests
permissions:
contents: read
packages: read
uses: ./.github/workflows/velox-compute-sanitizer-run.yaml
with:
tool_name: "synccheck"
test_names: ${{ needs.discover-tests.outputs.tests }}
velox_image: ${{ needs.discover-tests.outputs.velox_image }}
gh workflow run velox-compute-sanitizer-run.yaml "${args[@]}"

@misiugodfrey misiugodfrey Jun 25, 2026

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.

IIUC this job will now be green as long as the sub-jobs have been dispatched - regardless of whether they pass or not. Is this ok with how we intend to view/report these results?
I don't think we want to leave the triggering job spinning as it waits for the other jobs to finish; I just want to make sure we aren't going to miss failures if they do occur by looking at the wrong reporting. If those jobs are hooked into slack or something then that's probably fine.

@bdice bdice Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this succeeds if the trigger succeeds. I think that’s fine. We can’t spin-wait here because the actual runs take too long.

Loading