diff --git a/.github/scripts/ci-outage.sh b/.github/scripts/ci-outage.sh deleted file mode 100755 index bb13509e3..000000000 --- a/.github/scripts/ci-outage.sh +++ /dev/null @@ -1,118 +0,0 @@ -#!/bin/bash -# Per-cluster circuit breaker for environment-wide outages. -# -# Some failures are not the node's fault and not the code's fault: pypi.org -# unreachable from a login node, a module tree mid-upgrade, a full project -# filesystem. Requeuing elsewhere cannot help, and every job that starts pays -# the same discovery cost -- on 2026-08-28, 17 Frontier jobs each spent ~33 -# minutes learning that PyPI was down. -# -# The first job to notice records a marker on the shared filesystem (every -# self-hosted runner for a cluster shares $HOME); later jobs check it and exit -# immediately instead of submitting a SLURM job that is going to fail. -# -# The breaker is deliberately self-healing. A marker expires after -# MFC_CI_OUTAGE_TTL_SECONDS, and a marker that cannot be parsed is ignored, so -# neither a stale file nor a truncated write can wedge CI. Only jobs that -# actually observe the outage re-mark it, so once the outage clears the breaker -# closes on its own. -# -# Usage: -# ci-outage.sh mark record an outage -# ci-outage.sh check exit 0 = clear, 1 = outage active -# ci-outage.sh clear reset the breaker -# -# Env: -# MFC_CI_STATE_DIR where markers live (default ~/.mfc-ci-state) -# MFC_CI_OUTAGE_TTL_SECONDS marker lifetime in seconds (default 1200) - -set -uo pipefail - -STATE_DIR="${MFC_CI_STATE_DIR:-$HOME/.mfc-ci-state}" -TTL="${MFC_CI_OUTAGE_TTL_SECONDS:-1200}" - -# A non-numeric TTL would make the age comparison below emit "integer expression -# expected" and exit with a code the caller reads as neither clear nor tripped. -# Fall back to the default rather than letting a typo gate CI. -case "$TTL" in - ''|*[!0-9]*) - echo "Ignoring non-numeric MFC_CI_OUTAGE_TTL_SECONDS='$TTL'; using 1200." >&2 - TTL=1200 - ;; -esac - -EXIT_CLEAR=0 -EXIT_TRIPPED=1 -EXIT_USAGE=2 - -usage() { - echo "Usage: $0 {mark |check |clear }" >&2 -} - -# Keep the marker name filesystem-safe regardless of what the caller passes. -marker_for() { - local cluster - cluster=$(printf '%s' "$1" | tr -c 'A-Za-z0-9_.-' '_') - printf '%s/outage-%s' "$STATE_DIR" "$cluster" -} - -cmd="${1:-}" -cluster="${2:-}" - -if [ -z "$cmd" ] || [ -z "$cluster" ]; then - usage - exit $EXIT_USAGE -fi - -marker=$(marker_for "$cluster") - -case "$cmd" in - mark) - reason="${3:-unspecified}" - mkdir -p "$STATE_DIR" || exit $EXIT_USAGE - # Write to a temporary file and rename so a concurrent `check` never - # observes a half-written marker. - tmp="${marker}.$$.tmp" - { - date +%s - printf '%s\n' "$reason" - } > "$tmp" && mv -f "$tmp" "$marker" - echo "Recorded $cluster outage: $reason" - echo " marker: $marker (expires after ${TTL}s)" - ;; - - check) - [ -f "$marker" ] || exit $EXIT_CLEAR - - stamp=$(head -n1 "$marker" 2>/dev/null) - reason=$(tail -n +2 "$marker" 2>/dev/null) - - # A marker we cannot parse is treated as absent: an unreadable breaker - # must never be an un-clearable one. - case "$stamp" in - ''|*[!0-9]*) - echo "Ignoring unparseable outage marker $marker" - exit $EXIT_CLEAR - ;; - esac - - age=$(( $(date +%s) - stamp )) - if [ "$age" -ge "$TTL" ] || [ "$age" -lt 0 ]; then - exit $EXIT_CLEAR - fi - - echo "::warning::Skipping: known $cluster outage recorded ${age}s ago: ${reason:-unspecified}" - echo "Clear it early by deleting $marker" - exit $EXIT_TRIPPED - ;; - - clear) - rm -f "$marker" - echo "Cleared any $cluster outage marker ($marker)" - ;; - - *) - usage - exit $EXIT_USAGE - ;; -esac diff --git a/.github/scripts/classify-build-failure.sh b/.github/scripts/classify-build-failure.sh deleted file mode 100755 index 6f2e56e00..000000000 --- a/.github/scripts/classify-build-failure.sh +++ /dev/null @@ -1,41 +0,0 @@ -#!/bin/bash -# Decide whether a failed build was a cluster-wide dependency outage. -# -# MFC bootstraps its Python toolchain into build/venv on the first ./mfc.sh call -# of a job, pulling from pypi.org. On Phoenix clean_build has just moved build/ -# aside, so that happens every time; on Frontier it happens in the login-node -# "Fetch Dependencies" step. When the index is unreachable the build fails for a -# reason no other node improves on, so it is worth recording once and skipping -# the rest of the matrix rather than having each job spend ~33 minutes -# rediscovering it (17 Frontier jobs did exactly that on 2026-08-28). -# -# Usage: classify-build-failure.sh -# -# Exit codes: -# 78 cluster-wide dependency outage; it has been recorded -# 0 ordinary build failure, caller should keep its own exit code - -set -uo pipefail - -log="${1:-}" -cluster="${2:-}" - -if [ -z "$log" ] || [ -z "$cluster" ]; then - echo "Usage: $0 " >&2 - exit 0 -fi - -# No log means nothing to classify. Never claim an outage on absent evidence. -[ -f "$log" ] || exit 0 - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" - -# The URL may or may not be wrapped (uv quotes it in backticks, plain pip does -# not), so do not require a character between the colon and the scheme. -if grep -qE "Failed to fetch:[^h]*https?://pypi|uv install failed|\(venv\) Installation failed" "$log"; then - bash "$SCRIPT_DIR/ci-outage.sh" mark "$cluster" \ - "PyPI/uv dependency install failed during build" - exit 78 -fi - -exit 0 diff --git a/.github/scripts/monitor_slurm_job.sh b/.github/scripts/monitor_slurm_job.sh index 7d42e119a..d508306c5 100755 --- a/.github/scripts/monitor_slurm_job.sh +++ b/.github/scripts/monitor_slurm_job.sh @@ -35,6 +35,15 @@ output_file="$2" echo "Submitted batch job $job_id" echo "Monitoring output file: $output_file" +# Put the one thing a reader needs on the run's summary page. Without this, +# learning why a job failed means opening a log of tens of thousands of lines -- +# and an infrastructure fault looks exactly like a test failure until you do. +# Silent when not running under Actions. +ci_summary() { + [ -n "${GITHUB_STEP_SUMMARY:-}" ] || return 0 + printf '%b\n' "$1" >> "$GITHUB_STEP_SUMMARY" +} + # Robustly check SLURM job state using squeue with sacct fallback. # Returns the state string (PENDING, RUNNING, COMPLETED, FAILED, etc.) # or "UNKNOWN" if both commands fail. @@ -213,8 +222,14 @@ while true; do sleep "$MFC_MONITOR_POLL_SECONDS" done -# Give tail a moment to flush the final lines, then stop streaming. +# Give tail a moment to flush the final lines, then stop streaming. Whether it +# was still alive decides how much needs reprinting below: if it streamed the +# whole job, printing the file again just doubles every log. sleep 2 +streamed_ok=0 +if kill -0 "${tail_pid}" 2>/dev/null; then + streamed_ok=1 +fi kill "${tail_pid}" 2>/dev/null || true tail_pid="" @@ -238,9 +253,20 @@ if [ -f "$output_file" ]; then done fi +# Reprint only what streaming may have missed. `tail -f` above already emitted +# the whole file as it was written, so cat'ing it again duplicated every job's +# output -- measured at 3 copies of each line on a GPU job, and 65,000 lines of +# offload diagnostics repeated for a single fault. The reprint exists solely as +# a safety net for a tail that died mid-job, so it is bounded when tail survived +# and complete only when it did not. echo "" -echo "=== Final output ===" -cat "$output_file" +if [ "${streamed_ok:-0}" -eq 1 ]; then + echo "=== Final output (tail; the full log streamed above) ===" + tail -n "${MFC_MONITOR_FINAL_LINES:-40}" "$output_file" +else + echo "=== Final output (streaming stopped early; reprinting in full) ===" + cat "$output_file" +fi # Check exit status with sacct fallback exit_code="" @@ -267,26 +293,32 @@ if [ -z "$exit_code" ]; then exit 1 fi -# Infrastructure verdicts from the in-allocation preflight come back as the -# job's own exit code. Relay them verbatim: flattening them to 1 would leave the -# submit wrapper unable to tell "this node is unusable" (exclude it and try -# again) from "the tests failed" (report it). +# The preflight's node-fault verdict comes back as the job's own exit code. +# Relay it verbatim: flattening it to 1 would leave the submit wrapper unable to +# tell "this node is unusable" (exclude it and try again) from "the tests +# failed" (report it). +faulted_node=$(grep -oE 'MFC_FAULT_NODE=[^ ]+' "$output_file" 2>/dev/null | tail -n1 | cut -d= -f2 || true) + case "$exit_code" in 77:*) echo "Job $job_id failed preflight: the node is unusable — signaling caller to exclude it and resubmit." + ci_summary "### :warning: Infrastructure fault — not a code or test failure\n\nNode \`${faulted_node:-unknown}\` could not run MFC (job \`$job_id\`). It is excluded and the job resubmitted elsewhere.\n" monitor_success=1 exit 77 ;; - 78:*) - echo "Job $job_id skipped: a cluster-wide outage is already recorded." - monitor_success=1 - exit 78 - ;; esac # Check if job succeeded if [ "$exit_code" != "0:0" ]; then echo "ERROR: Job $job_id failed with exit code $exit_code" + # A GPU memory fault explains itself in a block the test harness prints; lift + # it onto the summary page so the faulting kernel and source line are visible + # without opening the log at all. + if grep -q 'GPU fault summary' "$output_file" 2>/dev/null; then + ci_summary "### GPU memory fault\n\n\`\`\`\n$(grep -A6 'GPU fault summary' "$output_file" | head -8 | sed 's/`/'"'"'/g')\n\`\`\`\n" + else + ci_summary "### Job \`$job_id\` failed (exit $exit_code)\n\n\`\`\`\n$(tail -n 15 "$output_file" | sed 's/`/'"'"'/g')\n\`\`\`\n" + fi exit 1 fi diff --git a/.github/scripts/preflight.sh b/.github/scripts/preflight.sh index e5e9f4a7b..ba0244f37 100755 --- a/.github/scripts/preflight.sh +++ b/.github/scripts/preflight.sh @@ -19,7 +19,6 @@ # Exit codes: # 0 node looks healthy, carry on # 77 node-local fault -- caller should exclude this node and resubmit -# 78 cluster-wide outage already recorded -- caller should skip, not requeue set -uo pipefail @@ -33,7 +32,6 @@ fi EXIT_HEALTHY=0 EXIT_NODE_FAULT=77 -EXIT_OUTAGE=78 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" node="${SLURMD_NODENAME:-$(hostname -s 2>/dev/null || hostname)}" @@ -50,19 +48,6 @@ if [ -z "${SLURM_JOB_ID:-}" ]; then exit $EXIT_HEALTHY fi -# --- Cluster-wide outage: requeuing cannot help, so skip rather than retry --- -outage_rc=0 -bash "$SCRIPT_DIR/ci-outage.sh" check "$cluster" || outage_rc=$? -if [ "$outage_rc" -eq 1 ]; then - echo "Preflight: skipping on $node because $cluster is known to be down." - exit $EXIT_OUTAGE -elif [ "$outage_rc" -ne 0 ]; then - # Only exit 1 means "tripped". Anything else means the breaker could not be - # read at all (missing script, unreadable state dir), which says nothing - # about the cluster -- treating it as an outage would halt CI on a bug here. - echo "Preflight: could not read the outage breaker (exit $outage_rc); continuing." -fi - # --- Node health --- # Pick the *newest* install matching this job's device (build/install is named # e.g. gpu-acc-, gpu-mp-). Both halves matter: the device filter @@ -114,7 +99,12 @@ echo "Preflight: probing $node with $syscheck_bin" # one there fails 127 no matter how healthy the node is. See # toolchain/templates/{phoenix,frontier,frontier_amd}.mako. case "$cluster" in - phoenix) launcher=(mpirun -np 1) ;; + # --bind-to none: a single-rank health probe has nothing to bind against, + # and Open MPI's default binding fails outright on some Phoenix nodes + # ("hwloc_set_cpubind returned Error for bitmap 0"), killing the process + # before the binary is even launched. That is a launcher problem, not a + # node problem -- but it condemned three healthy nodes before being caught. + phoenix) launcher=(mpirun --bind-to none -np 1) ;; frontier|frontier_amd) launcher=(srun -n1) ;; *) launcher=() ;; esac @@ -131,18 +121,53 @@ fi # PMIX_ERR_NO_PERMISSIONS and friends from dstore_base.c are benign and appear # in more passing jobs than failing ones, so matching on log text would fail # healthy nodes. -probe_rc=0 -if [ "${#launcher[@]}" -eq 0 ]; then - "$syscheck_bin" 2>&1 || probe_rc=$? -else - "${launcher[@]}" "$syscheck_bin" 2>&1 || probe_rc=$? -fi +# Captured to a variable, not a temp file: this runs before any module set is +# guaranteed and mktemp is not always on PATH here. +run_probe() { + probe_rc=0 + if [ "$#" -eq 0 ]; then + probe_out=$("$syscheck_bin" 2>&1) || probe_rc=$? + else + probe_out=$("$@" "$syscheck_bin" 2>&1) || probe_rc=$? + fi +} + +run_probe "${launcher[@]}" + +# If this launcher does not take the flags we added, drop them and probe again +# rather than reporting a verdict about the node. Otherwise a launcher that +# rejects an option would fail every probe, and -- because a failed launch is +# treated as inconclusive below -- would silently switch the preflight off +# instead of failing loudly. +case "$probe_out" in + *"unrecognized option"*|*"unrecognized argument"*|*"Unknown option"*|*"invalid option"*) + if [ "${#launcher[@]}" -gt 1 ]; then + echo "Preflight: ${launcher[0]} rejected the probe's options; retrying with none of them." + run_probe "${launcher[0]}" + fi + ;; +esac + +printf '%s\n' "$probe_out" if [ "$probe_rc" -eq 0 ]; then echo "Preflight: $node passed." exit $EXIT_HEALTHY fi +# Only a binary that RAN and failed says anything about this node. When the +# launcher never got as far as starting it, the verdict is about mpirun or the +# allocation, and excluding the node is both wrong and expensive -- three +# healthy Phoenix nodes were excluded this way, two jobs deep, before the run +# gave up. Judge nothing on a launch that never happened. +case "$probe_out" in + *"The specified application failed to start"*|*"unable to start the specified application"*|*"was killed without launching the target application"*) + echo "Preflight: the launcher could not start $syscheck_bin on $node;" + echo " that is a launcher or allocation problem, not evidence about the node. Continuing." + exit $EXIT_HEALTHY + ;; +esac + echo "::error::Preflight failed on $node: syscheck could not run MFC here." echo "This is an INFRASTRUCTURE fault, not a code or test failure." echo "MFC_FAULT_NODE=$node" diff --git a/.github/scripts/retry-build.sh b/.github/scripts/retry-build.sh index 60fc2559a..f6d5749aa 100755 --- a/.github/scripts/retry-build.sh +++ b/.github/scripts/retry-build.sh @@ -1,6 +1,8 @@ #!/bin/bash # Provides retry_build(): 2-attempt loop. -# On failure of attempt 1, nukes the entire build directory before attempt 2. +# On failure of attempt 1, nukes the build directory before attempt 2, keeping +# build/venv: a compute node cannot reinstall it (no route to PyPI), so removing +# it made every retry fail on a dependency fetch that could not succeed (#1813). # If RETRY_VALIDATE_CMD is set, runs it after a successful build; a non-zero # exit triggers the same nuke-and-retry, catching e.g. SIGILL from binaries # compiled on a different CPU architecture. @@ -12,6 +14,10 @@ # path without waiting on it; CI leaves it at the default. : "${MFC_BUILD_RETRY_DELAY:=30}" +nuke_build() { + find build -mindepth 1 -maxdepth 1 ! -name venv -exec rm -rf -- {} + 2>/dev/null || true +} + retry_build() { local max_attempts=2 local validate_cmd="${RETRY_VALIDATE_CMD:-}" @@ -23,8 +29,8 @@ retry_build() { if ! eval "$validate_cmd"; then echo "Post-build validation failed on attempt $attempt." if [ $attempt -lt $max_attempts ]; then - echo " Nuking build directory before retry..." - rm -rf build 2>/dev/null || true + echo " Clearing the build directory (keeping build/venv) before retry..." + nuke_build sleep 5 attempt=$((attempt + 1)) continue @@ -38,8 +44,8 @@ retry_build() { return 0 fi if [ $attempt -lt $max_attempts ]; then - echo " Build failed — nuking build directory before retry..." - rm -rf build 2>/dev/null || true + echo " Build failed — clearing the build directory (keeping build/venv) before retry..." + nuke_build sleep "$MFC_BUILD_RETRY_DELAY" else echo "Build failed after $max_attempts attempts." diff --git a/.github/scripts/run_case_optimization.sh b/.github/scripts/run_case_optimization.sh index 75ab3a44a..0a6bd1b44 100755 --- a/.github/scripts/run_case_optimization.sh +++ b/.github/scripts/run_case_optimization.sh @@ -102,6 +102,23 @@ for case in "${benchmarks[@]}"; do # its run is sharded across concurrent jobs sharing one workspace, so a # fallback rebuild would race on the shared install paths (the collision the # --no-build guard above prevents). + # The same offload diagnostics the test harness sets. These cases run on + # GPUs, and a memory fault here previously surfaced as a bare device + # address with nothing to act on. Both variables are inert until a fault; + # the debug agent is what gives CCE a faulting kernel at all, and is set + # only where its library is actually reachable. + # OFFLOAD_TRACK_ALLOCATION_TRACES / _NUM_KERNEL_LAUNCH_TRACES are deliberately + # NOT set: measured on an MI210 with amdflang, either one alone turns a + # 5.94 s test into a >400 s timeout, because they instrument every + # allocation and every kernel launch. See toolchain/mfc/gpu_diagnostics.py. + # Skipped when the caller already chose a tool, or is collecting a GPU core + # dump -- the agent is mutually exclusive with one, so loading it anyway + # would leave them with no dump and no reason why. + if [ -z "${HSA_TOOLS_LIB:-}" ] && [ -z "${HSA_ENABLE_DEBUG:-}" ] \ + && [ -n "${ROCM_PATH:-}" ] && [ -f "$ROCM_PATH/lib/librocm-debug-agent.so.2" ]; then + export HSA_TOOLS_LIB=librocm-debug-agent.so.2 + fi + run_log="$(mktemp)" ./mfc.sh run "$case" --case-optimization $gpu_opts $build_opts -n "$ngpus" -j 8 -c "$job_cluster" -- --gbpp 1 --steps 10 2>&1 | tee "$run_log" run_rc=${PIPESTATUS[0]} @@ -118,6 +135,14 @@ for case in "${benchmarks[@]}"; do else run_ok=0 fi + + # A fault's agent report runs to tens of thousands of lines and the useful + # part is in the middle, so re-print a bounded summary at the end where a + # reader will actually find it. Silent when the log has no agent report. + if [ "$run_ok" = 0 ]; then + build/venv/bin/python3 .github/scripts/summarize_gpu_fault.py "$run_log" || true + fi + rm -f "$run_log" if [ "$run_ok" = 1 ]; then diff --git a/.github/scripts/run_monitored_slurm_job.sh b/.github/scripts/run_monitored_slurm_job.sh index f18af2b55..16d57b0fc 100644 --- a/.github/scripts/run_monitored_slurm_job.sh +++ b/.github/scripts/run_monitored_slurm_job.sh @@ -29,18 +29,13 @@ if [ "$monitor_exit" -eq 76 ]; then exit 76 fi -# 77 (node fault) and 78 (recorded outage) are verdicts the preflight reached -# inside the allocation, not monitor failures — there is nothing to re-check -# with sacct, so relay them straight through rather than falling into the -# recovery path below. +# 77 (node fault) is a verdict the preflight reached inside the allocation, not +# a monitor failure — there is nothing to re-check with sacct, so relay it +# straight through rather than falling into the recovery path below. if [ "$monitor_exit" -eq 77 ]; then echo "Monitor reports SLURM job $job_id failed preflight — signaling caller to exclude the node and resubmit." exit 77 fi -if [ "$monitor_exit" -eq 78 ]; then - echo "Monitor reports SLURM job $job_id was skipped due to a recorded outage." - exit 78 -fi if [ "$monitor_exit" -ne 0 ]; then echo "Monitor exited with code $monitor_exit; re-checking SLURM job $job_id final state..." @@ -64,10 +59,6 @@ if [ "$monitor_exit" -ne 0 ]; then echo "SLURM job $job_id failed preflight — signaling caller to exclude the node and resubmit." exit 77 ;; - 78:*) - echo "SLURM job $job_id was skipped due to a recorded outage." - exit 78 - ;; esac if [ "$final_state" = "COMPLETED" ] && [ "$final_exit" = "0:0" ]; then echo "SLURM job $job_id completed successfully despite monitor failure — continuing." diff --git a/.github/scripts/submit-slurm-job.sh b/.github/scripts/submit-slurm-job.sh index f5480d176..5bdf5a404 100755 --- a/.github/scripts/submit-slurm-job.sh +++ b/.github/scripts/submit-slurm-job.sh @@ -195,20 +195,6 @@ rm -f "$output_file" # --- Module load mode (short form) --- module_mode=$([ "$device" = "gpu" ] && echo "g" || echo "c") -# --- Skip entirely if this cluster is already known to be down --- -# Checking only inside the allocation would mean every matrix job still pays the -# full queue wait -- hours on Phoenix 'embers' -- before finding the marker. Only -# exit 1 means "tripped"; any other failure means the breaker itself could not be -# read, which says nothing about the cluster. -outage_rc=0 -bash "${SCRIPT_DIR}/ci-outage.sh" check "$cluster" || outage_rc=$? -if [ "$outage_rc" -eq 1 ]; then - echo "::warning::Not submitting: $cluster is under a recorded outage." - exit 78 -elif [ "$outage_rc" -ne 0 ]; then - echo "Could not read the outage breaker (exit $outage_rc); submitting anyway." -fi - # --- Submit (with retries for transient SLURM errors) --- source "${SCRIPT_DIR}/retry-sbatch.sh" # Re-rendered before every submission so a node added to $node_exclude by a @@ -317,12 +303,6 @@ while :; do echo "That is a cluster-wide problem rather than a bad draw; not resubmitting." exit 1 fi - if [ "$monitor_rc" -eq 78 ]; then - # A recorded cluster-wide outage. Another node cannot help, so stop - # rather than spend more allocations proving the same point. - echo "::warning::Not resubmitting: $cluster is under a recorded outage." - exit "$monitor_rc" - fi # Genuine failure (not preemption or infrastructure). exit "$monitor_rc" done diff --git a/.github/scripts/summarize_gpu_fault.py b/.github/scripts/summarize_gpu_fault.py new file mode 100755 index 000000000..c8aee9949 --- /dev/null +++ b/.github/scripts/summarize_gpu_fault.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python3 +"""Print a bounded summary of a GPU memory fault in a run log. + +For callers that are shell scripts. The ROCm debug agent emits tens of +thousands of lines per fault -- one disassembly and register dump repeated per +faulting wave -- and the part worth reading (the faulting kernel, the fault +reason, the stop-PC distribution) is buried in the middle, so `tail` cannot +find it. + +Exits 0 having printed a summary, or 1 having printed nothing when the log has +no agent report, which lets the caller fall back to whatever it did before. +""" + +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "toolchain")) + +from mfc.gpu_diagnostics import summarize_rocm_debug_agent # noqa: E402 + + +def main() -> int: + if len(sys.argv) != 2: + print(f"usage: {sys.argv[0]} ", file=sys.stderr) + return 2 + + try: + with open(sys.argv[1], "r", encoding="utf-8", errors="replace") as log: + summary = summarize_rocm_debug_agent(log.read()) + except OSError as exc: + print(f"could not read {sys.argv[1]}: {exc}", file=sys.stderr) + return 1 + + if not summary: + return 1 + + print(summary) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index e8dd047f5..f35054e59 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -30,7 +30,10 @@ jobs: filters: ".github/file-filter.yml" self: - name: "${{ matrix.name }} (${{ matrix.device }}${{ matrix.interface != 'none' && format('-{0}', matrix.interface) || '' }})" + # Prefixed so it cannot be confused with the Test Suite job of the same + # cluster and device: the two produced identical check names, and a + # benchmark failure repeatedly read as a test-suite regression. + name: "Bench | ${{ matrix.name }} (${{ matrix.device }}${{ matrix.interface != 'none' && format('-{0}', matrix.interface) || '' }})" if: ${{ github.repository=='MFlowCode/MFC' && needs.file-changes.outputs.checkall=='true' && github.event.pull_request.draft != true && ((github.event_name=='pull_request_review' && github.event.review.state=='approved') || (github.event_name=='pull_request' && (github.event.pull_request.user.login=='sbryngelson' || github.event.pull_request.user.login=='wilfonba')) || github.event_name=='workflow_dispatch') }} needs: file-changes strategy: diff --git a/.github/workflows/common/build.sh b/.github/workflows/common/build.sh index b6cf2e14b..8e8ed7d80 100755 --- a/.github/workflows/common/build.sh +++ b/.github/workflows/common/build.sh @@ -75,11 +75,6 @@ run_build_step() { local rc=${PIPESTATUS[0]} set -e if [ "$rc" -ne 0 ]; then - local cls=0 - bash .github/scripts/classify-build-failure.sh "$log" "$job_cluster" || cls=$? - if [ "$cls" -ne 0 ]; then - exit "$cls" - fi exit "$rc" fi } @@ -98,8 +93,8 @@ run_build_step "${log_base}-syscheck.log" retry_build ./mfc.sh build -t syscheck preflight_rc=0 bash .github/scripts/preflight.sh "$job_cluster" "$job_device" || preflight_rc=$? if [ "$preflight_rc" -ne 0 ]; then - # 77 (bad node) and 78 (recorded outage) travel back to submit-slurm-job.sh - # as this job's exit code, which decides whether to requeue elsewhere. + # 77 (bad node) travels back to submit-slurm-job.sh as this job's exit code, + # which decides whether to requeue elsewhere. exit "$preflight_rc" fi diff --git a/.github/workflows/frontier/build.sh b/.github/workflows/frontier/build.sh index 4ad359a4b..747578c9f 100644 --- a/.github/workflows/frontier/build.sh +++ b/.github/workflows/frontier/build.sh @@ -24,20 +24,16 @@ clean_build source .github/scripts/retry-build.sh -# This login-node step is where Frontier's dependency install actually happens, -# and so where a PyPI outage actually lands -- 17 jobs spent ~33 minutes each -# rediscovering one on 2026-08-28. Tee the output and classify a failure so the -# first job to hit it records it and the rest of the matrix can skip. +# Frontier's dependency install happens here, on the login node -- so a failed +# download costs no allocation and there is nothing to protect the matrix from. +# A flaky PyPI fetch used to be recorded as a cluster-wide outage, which then +# skipped every other job on that cluster: one bad download turned into a red +# matrix, including jobs whose tests had already passed. uv already retries. # No set -e in this script, so capture the status rather than toggling it. deps_log="deps-${cluster_name}-${job_device}-${job_interface}.log" retry_build ./mfc.sh build --deps-only -j 8 $build_opts 2>&1 | tee "$deps_log" deps_rc=${PIPESTATUS[0]} if [ "$deps_rc" -ne 0 ]; then - cls=0 - bash .github/scripts/classify-build-failure.sh "$deps_log" "$cluster_name" || cls=$? - if [ "$cls" -ne 0 ]; then - exit "$cls" - fi exit 1 fi diff --git a/toolchain/mfc/bench.py b/toolchain/mfc/bench.py index 68146d5aa..daf3cb501 100644 --- a/toolchain/mfc/bench.py +++ b/toolchain/mfc/bench.py @@ -12,6 +12,7 @@ from .build import DEFAULT_TARGETS, SIMULATION, get_targets from .common import MFC_BENCH_FILEPATH, MFC_BUILD_DIR, MFCException, console_safe, create_directory, file_dump_yaml, file_load_yaml, format_list_to_string, log_tail, system +from .gpu_diagnostics import fault_diagnostic_env, summarize_rocm_debug_agent from .printer import cons from .state import ARG, CFG @@ -23,6 +24,25 @@ class BenchCase: args: typing.List[str] +def bench_failure_report(log_filepath: str) -> str: + """What to show for a failed benchmark case. + + A GPU memory fault under the ROCm debug agent runs to tens of thousands of + lines, nearly all of it one disassembly and register dump repeated per wave. + A fixed tail is not merely long here, it is wrong: measured on a real + report, the last 80 lines are a single wave's registers and the kernel name + -- the only part worth having -- is not among them. Fall back to the tail + only when there is no agent report to summarize. + """ + try: + with open(log_filepath, "r", encoding="utf-8", errors="replace") as log_file: + summary = summarize_rocm_debug_agent(log_file.read()) + except OSError: + return log_tail(log_filepath) + + return summary or log_tail(log_filepath) + + def bench(targets=None): if targets is None: targets = ARG("targets") @@ -76,6 +96,10 @@ def bench(targets=None): ["./mfc.sh", "run", case.path] + ["--targets"] + [t.name for t in targets] + ["--output-summary", summary_filepath] + case.args + ["--", "--gbpp", str(ARG("mem"))], stdout=log_file, stderr=subprocess.STDOUT, + # Same offload diagnostics the test harness uses: + # these cases run on GPUs too, and a fault here + # was previously reported as a bare address. + env=fault_diagnostic_env(dict(os.environ)), ) # Check return code (handle CompletedProcess or int defensively) @@ -89,7 +113,7 @@ def bench(targets=None): cons.print(f"[bold red]ERROR[/bold red]: Case {case.slug} failed with exit code {rc}") # Print the log, not just its path: this file lives # on the cluster and no artifact upload collects it. - cons.print(console_safe(log_tail(log_filepath))) + cons.print(console_safe(bench_failure_report(log_filepath))) failed_cases.append(case.slug) break @@ -101,7 +125,7 @@ def bench(targets=None): time.sleep(5) continue cons.print(f"[bold red]ERROR[/bold red]: Summary file not created for {case.slug}") - cons.print(console_safe(log_tail(log_filepath))) + cons.print(console_safe(bench_failure_report(log_filepath))) cons.print(f"[bold red] Expected: {summary_filepath}[/bold red]") failed_cases.append(case.slug) break diff --git a/toolchain/mfc/gpu_diagnostics.py b/toolchain/mfc/gpu_diagnostics.py new file mode 100644 index 000000000..ecfdb0124 --- /dev/null +++ b/toolchain/mfc/gpu_diagnostics.py @@ -0,0 +1,200 @@ +"""Offload-runtime diagnostics for GPU memory faults. + +Shared by the test harness, the benchmark runner and the case-optimization CI +script -- all three run GPU cases and all three need the same answer when one +faults. Kept out of test/ because bench.py depending on the test module to +explain a crash would be the wrong way round. +""" + +import collections +import os +import re +import typing + +# The marker _handle_case attaches to the exception it raises, so that +# classify_error can tell a GPU memory fault from any other execution failure. +# Two constraints, both learned the hard way: +# +# * It must be one of the signatures below verbatim, because classify_error +# recognises it by running the same matcher over the message. An earlier +# version wrote "[gpu-memory-fault]" while the reader searched for "memory +# access fault by gpu", so the two never matched and the feature was dead +# while seven source-inspecting tests passed. +# * No square brackets. main.py renders these messages through Rich, which +# parses "[...]" as a style tag and deletes it -- which is why a CI log +# showed a bare "Failed to execute MFC. " with the marker missing. +GPU_FAULT_MARKER = "(memory access fault by GPU)" + +GPU_FAULT_SIGNATURES = ( + # AMD/HSA -- Frontier, both CCE and AFAR builds. + "memory access fault by gpu", + "offload error: memory access fault", + # NVHPC -- Phoenix. Worded nothing like the AMD ones, so matching only the + # above meant 189 faults on a Phoenix gpu-acc shard were never recognised. + # Only the specific error: NVHPC prefixes unrelated failures with + # "Accelerator Fatal Error" too, including "call to cuMemAlloc returned + # error 2: Out of memory", which is not a memory fault and must not be + # classified as one. + "cuda_error_illegal_address", +) + + +def is_gpu_memory_fault(text: str) -> bool: + """Whether output shows a GPU memory fault, as opposed to any other failure. + + Deliberately narrow. PMIX_ERR_NO_PERMISSIONS and friends appear in 16% of + *passing* self-hosted jobs, so anything broader would fire constantly. + """ + lowered = (text or "").lower() + + return any(sig in lowered for sig in GPU_FAULT_SIGNATURES) + + +def fault_diagnostic_env(base: dict) -> dict: + """`base` plus the one diagnostic cheap enough to leave on. + + Set on every run rather than on a retry: the ROCm debug agent writes nothing + until the runtime is already aborting on a memory fault, so a first failure + is explained without spending a second run to reproduce it. + + Two variables that used to live here were removed after measurement -- see + below. What is left is the agent, which is what names the faulting kernel. + """ + env = dict(base) + + # OFFLOAD_TRACK_ALLOCATION_TRACES and OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES + # were set here and had to be removed. They instrument every allocation and + # every kernel launch, so a healthy run pays continuously: on an MI210 with + # amdflang, test AFBCBDFA takes 5.94 s with neither and times out past 400 s + # with either one alone -- enough, against the 1-hour test timeout, to turn + # a fault into a timeout and hide what they exist to explain. + # + # They looked free only because the A/B that cleared them ran on CCE, whose + # offload runtime ignores libomptarget variables entirely. What they added, + # one line on whether the address was ever a real allocation, the agent's + # kernel name and source line subsume. + + # Skipped when the caller is already debugging by hand: they chose a tool, + # or they set HSA_ENABLE_DEBUG to collect a GPU core dump, which the agent + # is mutually exclusive with. Loading it anyway would leave them with + # "Failed to enable debug interface" and no dump. An attached rocgdb trips + # the same path. + # + # Cost, Frontier CCE --gpu mp over four interleaved pairs: no effect + # detected on a healthy run (resolution ~0.8%), no output at all until + # something faults, and +0.387 s on a faulting run -- 0.011% of the test + # timeout. ~3-4% on an MI210 (n=2). It does not supersede libomptarget's own + # report on the AFAR lane; it is exclusive with ROCr core dumps only. + if "HSA_TOOLS_LIB" not in env and not env.get("HSA_ENABLE_DEBUG") and rocm_debug_agent_path() is not None: + env["HSA_TOOLS_LIB"] = ROCM_DEBUG_AGENT + + return env + + +ROCM_DEBUG_AGENT = "librocm-debug-agent.so.2" + + +def rocm_debug_agent_path() -> typing.Optional[str]: + """Where the ROCm debug agent lives, or None if it is not reachable. + + MUST be evaluated at call time, never cached at import. On Frontier the + library is on disk the whole time, but /opt/rocm-*/lib only reaches + LD_LIBRARY_PATH once `mfc.sh load` runs. A gate evaluated at import decides + "absent" on the one machine this exists for, and does it indistinguishably + from the Phoenix case where the library really is missing. + + Probes for the file rather than dlopen'ing it: ctypes.CDLL would load a + debug agent into the test harness's own process to answer a question about + the subprocess. + """ + rocm_path = os.environ.get("ROCM_PATH", "") + search = [os.path.join(rocm_path, "lib")] if rocm_path else [] + search += os.environ.get("LD_LIBRARY_PATH", "").split(os.pathsep) + + for directory in search: + if directory and os.path.isfile(os.path.join(directory, ROCM_DEBUG_AGENT)): + return os.path.join(directory, ROCM_DEBUG_AGENT) + + return None + + +def summarize_rocm_debug_agent(out: str, max_disasm: int = 14) -> str: + """Collapse librocm-debug-agent output to a bounded, informative summary. + + The agent repeats an identical disassembly block and a 115-line register + dump per faulting wave -- 125 waves produced 14,635 lines on a 49x39 case. + Only the kernel name, fault reason, stop-PC distribution and one + representative wave carry information; the rest is duplicated. + + A fixed tail cannot substitute. Measured on that log: the first 80 lines are + one wave's registers and the last 80 are another's, and the kernel name -- + the entire point -- appears in neither. The stop-PC histogram is kept + because the waves halted at four distinct PCs whose modal one is a load + while the injected fault is a write, so quoting a single PC without the + distribution hands the reader the wrong instruction. + + Returns '' when there is no agent report, so callers fall back to the raw + output. + + The format is NOT stable across ROCm versions, and the failure is silent -- + no wave match means an empty summary and a fallback to tens of thousands of + raw lines, with nothing saying why. Measured between two versions: + + 6.3.1 wave_124: pc=0x7ff77e253408 (stopped, reason: MEMORY_VIOLATION) + 7.2.0 wave_250: pc=0x7ff734dcbf3c (kernel_code_entry=0x... <...>, + kernargs=0x...) (stopped, reason: MEMORY_VIOLATION) + + 6.3.1 Memory access fault by GPU node-4 (Agent handle: ...) on address + 7.2.0 OFFLOAD ERROR: memory access fault by GPU 4 (agent ...) at ... + + An earlier version required pc= and "(stopped, reason:" to be adjacent and + matched the fault line case-sensitively on "Memory". It returned nothing at + all for 65,210 lines of real 7.2.0 output. Hence the tolerant separator, and + reusing is_gpu_memory_fault rather than hardcoding one version's wording. + Both formats are pinned by fixtures below. + + Validated against three real reports, not one: + + CCE acc ROCm 6.3.1 14,635 lines -> 37 + CCE mp ROCm 6.3.1 13,826 lines -> 35 + AFAR mp ROCm 7.2.0 65,210 lines -> 36 + + and output from a run with no agent loaded still yields '', so the fallback + is intact. The stop-PC histogram earns its place most on the CCE OpenMP + lane, which halts at seven distinct PCs (62/21/19/10/10/2/1) against four + for CCE OpenACC and one for AFAR: quoting a single PC would be wrong there + six times in seven. + """ + waves = re.findall(r"^wave_\d+: pc=(0x[0-9a-f]+).*?\(stopped, reason: (\w+)\)", out, re.M) + if not waves: + return "" + + lines = out.splitlines() + fault = next((line for line in lines if is_gpu_memory_fault(line)), None) + kernels = sorted({m.group(1) for m in re.finditer(r"^Disassembly for function (.+):$", out, re.M)}) + pcs = collections.Counter(pc for pc, _ in waves) + reasons = collections.Counter(reason for _, reason in waves) + + summary = [f"=== GPU fault summary (rocm-debug-agent, {len(lines)} lines collapsed) ==="] + if fault: + summary.append(fault.strip()) + summary.append("faulting kernel(s): " + (", ".join(kernels) or "")) + summary.append(f"faulting waves: {len(waves)} [" + ", ".join(f"{r} x{n}" for r, n in reasons.most_common()) + "]") + summary.append("stop PCs: " + ", ".join(f"{pc} x{n}" for pc, n in pcs.most_common())) + summary.append("NOTE: waves halt on fault detection, so the PC is near -- not necessarily at -- the offending instruction.") + + disasm_starts = [n for n, line in enumerate(lines) if line.startswith("Disassembly for function")] + if disasm_starts: + start = disasm_starts[0] + end = next((n for n, line in enumerate(lines[start:], start) if line.startswith("End of disassembly")), start + max_disasm) + summary += ["", f"--- disassembly (1 of {len(disasm_starts)} identical blocks) ---"] + summary += lines[start : min(end + 1, start + max_disasm)] + + modal_pc = pcs.most_common(1)[0][0] + start = next((n for n, line in enumerate(lines) if re.match(r"^wave_\d+: pc=" + re.escape(modal_pc) + r"(?![0-9a-f])", line)), None) + if start is not None: + summary += ["", f"--- representative wave (modal PC {modal_pc}, {pcs[modal_pc]} of {len(waves)} waves) ---"] + summary += lines[start : start + max_disasm] + summary.append(f" ... (registers for {len(waves) - 1} further waves suppressed)") + + return "\n".join(summary) diff --git a/toolchain/mfc/test/case.py b/toolchain/mfc/test/case.py index 14dfc4ef9..0b8835250 100644 --- a/toolchain/mfc/test/case.py +++ b/toolchain/mfc/test/case.py @@ -172,7 +172,7 @@ def __init__( merge = {key: val for key, val in merge.items() if val is not None} super().__init__(merge) - def run(self, targets: List[Union[str, MFCTarget]], gpus: Set[int]) -> subprocess.CompletedProcess: + def run(self, targets: List[Union[str, MFCTarget]], gpus: Set[int], env: dict = None) -> subprocess.CompletedProcess: if gpus is not None and len(gpus) != 0: gpus_select = ["--gpus"] + [str(_) for _ in gpus] else: @@ -192,9 +192,11 @@ def run(self, targets: List[Union[str, MFCTarget]], gpus: Set[int]) -> subproces command = [mfc_script, "run", filepath, "--no-build", *tasks, *case_optimization, *jobs, "-t", *target_names, *gpus_select, *ARG("--")] - return common.system(command, print_cmd=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + # env is per-subprocess, never os.environ: cases run in worker threads, + # so a mutated global would leak into every concurrent case. + return common.system(command, print_cmd=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env=env) - def run_restart(self, targets, gpus): + def run_restart(self, targets, gpus, env: dict = None): """Run a restart roundtrip: simulate to midpoint, then restart to end.""" # NOTE: This method overrides t_step_save to produce exactly one save # per phase (at the boundary step). Tests using restart_check=True @@ -213,7 +215,7 @@ def run_restart(self, targets, gpus): # Phase 1: Run to midpoint (generates restart data) self.params = {**orig, "t_step_stop": mid_step, "t_step_save": mid_step - orig["t_step_start"]} self.create_directory() - result1 = self.run(targets, gpus) + result1 = self.run(targets, gpus, env=env) if result1.returncode != 0: return result1 @@ -225,7 +227,7 @@ def run_restart(self, targets, gpus): # is run — it reads grid + IC directly from p_all/p0//. self.params = {**orig, "t_step_start": mid_step, "t_step_save": orig["t_step_stop"] - mid_step} self.create_directory() - result2 = self.run([SIMULATION], gpus) + result2 = self.run([SIMULATION], gpus, env=env) # Remove intermediate step files from D/ so only step 0 and # t_step_stop remain, matching the straight run's output. diff --git a/toolchain/mfc/test/test.py b/toolchain/mfc/test/test.py index 05720522d..f6320aa97 100644 --- a/toolchain/mfc/test/test.py +++ b/toolchain/mfc/test/test.py @@ -16,6 +16,13 @@ from .. import common, sched from ..build import HDF5, POST_PROCESS, PRE_PROCESS, SIMULATION, build from ..common import MFCException, console_safe, does_command_exist, format_list_to_string, get_program_output, log_tail +from ..gpu_diagnostics import ( + GPU_FAULT_MARKER, + fault_diagnostic_env, + is_gpu_memory_fault, + rocm_debug_agent_path, + summarize_rocm_debug_agent, +) from ..packer import packer from ..packer import tol as packtol from ..printer import cons @@ -620,7 +627,7 @@ def _handle_case(case: TestCase, devices: typing.Set[int]): # Check timeout before starting if timeout_flag.is_set(): raise TestTimeoutError("Test case exceeded 1 hour timeout") - cmd = case.run([PRE_PROCESS, SIMULATION], gpus=devices) + cmd = case.run([PRE_PROCESS, SIMULATION], gpus=devices, env=fault_diagnostic_env(dict(os.environ))) # Check timeout after simulation if timeout_flag.is_set(): @@ -631,7 +638,33 @@ def _handle_case(case: TestCase, devices: typing.Set[int]): common.file_write(out_filepath, cmd.stdout) if cmd.returncode != 0: - cons.print(cmd.stdout) + # The debug agent emits ~14k lines per fault, nearly all of it the + # same disassembly and register dump repeated per wave. Print the + # summary when there is one; the full capture is in out_pre_sim.txt. + agent_summary = summarize_rocm_debug_agent(cmd.stdout) + if agent_summary: + cons.print(console_safe(agent_summary)) + cons.print(f" full offload report: {out_filepath}") + else: + cons.print(cmd.stdout) + # Falling back is silent by nature: the raw output is printed + # and nothing says the summary was expected. That is exactly how + # a ROCm 6.3.1-only parser sat on the AFAR lane returning + # nothing for 65,210 lines of real 7.2.0 output. If the agent is + # reachable and this is a GPU fault, a missing summary means the + # agent did not load or its format moved again -- say so. + if is_gpu_memory_fault(cmd.stdout) and rocm_debug_agent_path() is not None: + cons.print( + " [yellow]warning[/yellow]: the ROCm debug agent is available and this is a GPU " + "memory fault, but no agent report was recognised -- the agent did not load, the run " + "was killed before it finished writing (a report is tens of thousands of lines), or " + "its output format has changed and summarize_rocm_debug_agent needs updating." + ) + # Marked so classify_error buckets it as a GPU memory fault rather + # than a generic execution failure; the diagnostics that make it + # actionable are already in the output above. + if is_gpu_memory_fault(cmd.stdout): + raise MFCException(f"Test {case}: Failed to execute MFC {GPU_FAULT_MARKER}.") raise MFCException(f"Test {case}: Failed to execute MFC.") _assert_particle_cloud_ib_state(case) @@ -676,7 +709,7 @@ def _handle_case(case: TestCase, devices: typing.Set[int]): if timeout_flag.is_set(): raise TestTimeoutError("Test case exceeded 1 hour timeout") - restart_result = case.run_restart([PRE_PROCESS, SIMULATION], devices) + restart_result = case.run_restart([PRE_PROCESS, SIMULATION], devices, env=fault_diagnostic_env(dict(os.environ))) if timeout_flag.is_set(): raise TestTimeoutError("Test case exceeded 1 hour timeout") @@ -753,6 +786,10 @@ def classify_error(exc: Exception) -> str: return "timeout" if "nan" in text: return "NaN detected" + # Before the generic branch: a GPU fault's message also contains "failed to + # execute", and it is the one execution failure a retry provably cannot fix. + if is_gpu_memory_fault(text): + return "GPU memory fault" if "failed to execute" in text: return "execution failed" diff --git a/toolchain/mfc/test/test_gpu_fault_diagnostics.py b/toolchain/mfc/test/test_gpu_fault_diagnostics.py new file mode 100644 index 000000000..cd2a4d856 --- /dev/null +++ b/toolchain/mfc/test/test_gpu_fault_diagnostics.py @@ -0,0 +1,310 @@ +"""GPU memory faults should explain themselves the first time. + +A fault reaches CI as an address and, unaided, nothing else. Measured against a +deliberate out-of-bounds write at m_time_steppers.fpp:486, on all four GPU +lanes: NVHPC and AFAR name the faulting kernel for free, CCE names nothing and +no CRAY_ACC_* variable helps, and the ROCm debug agent names it everywhere it +is reachable -- at ROCr level, with no recompile. + +Nearly every failure in this area was silent, so these assert on content and on +the hand-offs between parts, never on presence alone. +""" + +import pathlib +import subprocess +import sys + +from mfc.gpu_diagnostics import ( + GPU_FAULT_MARKER, + ROCM_DEBUG_AGENT, + fault_diagnostic_env, + is_gpu_memory_fault, + rocm_debug_agent_path, + summarize_rocm_debug_agent, +) + +# Real agent output, verbatim, from two ROCm versions. The formats differ in +# ways that silently defeated a parser written against only one: 7.2.0 puts +# kernel_code_entry=/kernargs= between pc= and "(stopped, reason:", and words +# the fault line "OFFLOAD ERROR: memory access fault ... at virtual address". +ROCM_AGENT_FIXTURE_631 = """\ +Memory access fault by GPU node-4 (Agent handle: 0x3b24f40) on address 0x7ffb6a0f6000. Reason: Write access to a read-only page. +Disassembly for function s_tvd_rk$m_time_steppers_$ck_L486_6: + code object: file:///path/gpu-acc-c819d00b45/bin/simulation#offset=4657152&size=10843816 + loaded at: [0x7ff77da00000-0x7ff77feca9f9] + => 0x7ff77e253430 <+6192>: s_waitcnt vmcnt(0) lgkmcnt(0) + 0x7ff77e253434 <+6196>: v_sub_co_u32_e32 v5, vcc, v26, v42 +End of disassembly. +wave_0: pc=0x7ff77e253408 (stopped, reason: MEMORY_VIOLATION) + +scalar registers: + s0: d9800000 s1: 80007ffe +wave_1: pc=0x7ff77e253408 (stopped, reason: MEMORY_VIOLATION) + +scalar registers: + s0: d9800000 s1: 80007ffe +wave_2: pc=0x7ff77e2533fc (stopped, reason: MEMORY_VIOLATION) + +scalar registers: + s0: d9800000 s1: 80007ffe +""" + +ROCM_AGENT_FIXTURE_720 = """\ +OFFLOAD ERROR: memory access fault by GPU 4 (agent 0x55555896f810) at virtual address 0x7ffb61f02000. Reasons: Write access to a read-only page +Disassembly for function __omp_offloading_8116438_1c00689b__QMm_time_steppersPs_tvd_rk_l486: + code object: memory://415882#offset=0x7ff736b8c040&size=17250800 + loaded at: [0x7ff734000000-0x7ff736b0a4e8] + => 0x7ff734dcbf3c <+7484>: s_waitcnt vmcnt(0) lgkmcnt(0) + 0x7ff734dcbf40 <+7488>: v_mul_f64 v[12:13], v[52:53], v[16:17] +End of disassembly. +wave_249: pc=0x7ff734dcbf3c (kernel_code_entry=0x7ff736b8c040 <__omp_offloading_8116438_1c00689b__QMm_time_steppersPs_tvd_rk_l486>, kernargs=0x7ffb61f00000) (stopped, reason: MEMORY_VIOLATION) + +scalar registers: + s0: d9800000 s1: 80007ffe +wave_250: pc=0x7ff734dcbf3c (kernel_code_entry=0x7ff736b8c040 <__omp_offloading_8116438_1c00689b__QMm_time_steppersPs_tvd_rk_l486>, kernargs=0x7ffb61f00000) (stopped, reason: MEMORY_VIOLATION) + +scalar registers: + s0: d9800000 s1: 80007ffe +""" + + +# Detection. + + +def test_recognises_each_runtime_wording(): + # AMD/HSA (CCE), libomptarget (AFAR), and NVHPC, which words it nothing like + # the others -- matching only AMD's left 189 Phoenix faults unclassified. + assert is_gpu_memory_fault("Memory access fault by GPU node-9 on address 0x1") + assert is_gpu_memory_fault("OFFLOAD ERROR: memory access fault by GPU 4") + assert is_gpu_memory_fault("Accelerator Fatal Error: ... (CUDA_ERROR_ILLEGAL_ADDRESS)") + + +def test_does_not_fire_on_ordinary_failures(): + # PMIX noise appears in 16% of *passing* self-hosted jobs, and NVHPC uses + # "Accelerator Fatal Error" for out-of-memory too -- neither is a fault. + assert not is_gpu_memory_fault("Test x: Failed to execute MFC.") + assert not is_gpu_memory_fault("PMIX ERROR: PMIX_ERR_NO_PERMISSIONS in file dstore_base.c") + assert not is_gpu_memory_fault("Accelerator Fatal Error: call to cuMemAlloc returned error 2: Out of memory") + + +def test_the_marker_survives_the_hand_off_and_rich(): + """The failure site tags the exception; classify_error reads the tag back. + + The first version wrote "[gpu-memory-fault]" while the reader searched for + "memory access fault by gpu", so the two never matched and the feature was + dead while seven source-inspecting tests passed. Square brackets also make + Rich delete the marker as a style tag before it reaches the log. + """ + import io + + from rich.console import Console + + raised = f"Test x: Failed to execute MFC {GPU_FAULT_MARKER}." + assert is_gpu_memory_fault(raised) + + console = Console(file=io.StringIO(), force_terminal=False) + console.print(raised) + assert GPU_FAULT_MARKER in console.file.getvalue() + + +def test_a_gpu_fault_gets_its_own_failure_class(): + # Otherwise detecting it is inert: classify_error bucketed anything saying + # "failed to execute" as a generic execution failure. + from mfc.common import MFCException + from mfc.test.test import classify_error + + assert classify_error(MFCException(f"Test x: Failed to execute MFC {GPU_FAULT_MARKER}.")) == "GPU memory fault" + assert classify_error(MFCException("Test x: Failed to execute MFC.")) == "execution failed" + + +# What the run environment carries. + + +def test_only_the_agent_is_set(): + """Everything else was measured to be worse than nothing. + + CRAY_ACC_DEBUG named the wrong kernel in 81 of 102 traced faults, because + CCE dispatches async and its trace's tail is whatever ran next. + OFFLOAD_TRACK_ALLOCATION_TRACES and _NUM_KERNEL_LAUNCH_TRACES instrument + every allocation and every kernel launch: on an MI210 either one alone + turned a 5.94 s test into a >400 s timeout. + """ + env = fault_diagnostic_env({}) + + assert "CRAY_ACC_DEBUG" not in env + assert "OFFLOAD_TRACK_ALLOCATION_TRACES" not in env + assert "OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES" not in env + + +def test_the_env_is_a_copy_and_keeps_what_it_was_given(): + # Cases run in worker threads; mutating a shared environment would leak + # settings into every concurrent case. + base = {"PATH": "/usr/bin", "HOME": "/home/x"} + env = fault_diagnostic_env(base) + + assert env["PATH"] == "/usr/bin" and env["HOME"] == "/home/x" + assert "HSA_TOOLS_LIB" not in base + + +def tmp_agent_dir() -> str: + """A directory laid out like a ROCm install, for the gate to find.""" + import os + import tempfile + + root = tempfile.mkdtemp() + os.makedirs(os.path.join(root, "lib"), exist_ok=True) + open(os.path.join(root, "lib", ROCM_DEBUG_AGENT), "w", encoding="utf-8").close() + return root + + +def test_the_agent_gate_re_reads_the_environment(monkeypatch): + """It must not be captured at import. + + On Frontier the library is on disk the whole time but only reaches + LD_LIBRARY_PATH once `mfc.sh load` runs, so an import-time gate reports + "absent" on the one machine this is for -- indistinguishably from Phoenix, + where it genuinely is missing. Pinned rather than trusting the host, which + may have a real ROCm install. + """ + import os + + monkeypatch.setenv("ROCM_PATH", "") + monkeypatch.setenv("LD_LIBRARY_PATH", "") + assert rocm_debug_agent_path() is None + assert "HSA_TOOLS_LIB" not in fault_diagnostic_env({}) + + monkeypatch.setenv("ROCM_PATH", tmp_agent_dir()) + assert rocm_debug_agent_path() is not None + assert fault_diagnostic_env({})["HSA_TOOLS_LIB"] == ROCM_DEBUG_AGENT + + monkeypatch.setenv("ROCM_PATH", "") + monkeypatch.setenv("LD_LIBRARY_PATH", os.path.join(tmp_agent_dir(), "lib")) + assert rocm_debug_agent_path() is not None + + +def test_a_developer_debugging_by_hand_is_left_alone(monkeypatch): + """`mfc.sh test` and `mfc.sh bench` are not only CI entry points. + + The agent is mutually exclusive with a ROCr core dump, so enabling it behind + someone collecting one gives them "Failed to enable debug interface" and no + dump, caused by the harness rather than anything they did. + """ + monkeypatch.setenv("ROCM_PATH", tmp_agent_dir()) + + assert "HSA_TOOLS_LIB" in fault_diagnostic_env({}) + assert "HSA_TOOLS_LIB" not in fault_diagnostic_env({"HSA_ENABLE_DEBUG": "1"}) + assert fault_diagnostic_env({"HSA_TOOLS_LIB": "libmine.so"})["HSA_TOOLS_LIB"] == "libmine.so" + + +# Collapsing the agent's output. + + +def test_both_rocm_formats_are_recognised(): + """A parser written against one version returns '' for the other. + + That happened: 65,210 lines of real 7.2.0 output produced nothing, on the + lane the summarizer exists to serve, with no error to explain it. Neither + format may be fixed at the other's expense. + """ + for name, fixture in (("6.3.1", ROCM_AGENT_FIXTURE_631), ("7.2.0", ROCM_AGENT_FIXTURE_720)): + summary = summarize_rocm_debug_agent(fixture) + assert summary, f"ROCm {name} agent output was not recognised" + assert "memory access fault" in summary.lower() + + +def test_the_summary_keeps_the_kernel_and_the_whole_pc_histogram(): + """The two things a fixed tail cannot give. + + On the real 14,635-line report the first 80 lines are one wave's registers + and the last 80 another's, so the kernel name is in neither. And the waves + stop at several PCs whose modal one is a load while the fault is a write -- + quoting one PC alone names the wrong instruction. + """ + summary = summarize_rocm_debug_agent(ROCM_AGENT_FIXTURE_720) + + assert "__omp_offloading_8116438_1c00689b__QMm_time_steppersPs_tvd_rk_l486" in summary + assert "0x7ff734dcbf3c x2" in summary + + for field in ("faulting kernel(s): ", "faulting waves: ", "stop PCs: ", "--- disassembly (1 of "): + assert field in summary, f"the summarizer no longer emits {field!r}" + + +def test_every_measured_symbol_form_survives(): + """Three manglings -- one per compiler, not one per offload model. + + CCE emits the same scheme for OpenACC and OpenMP offload, differing only in + a trailing counter, while AFAR's Flang form is different again. Reading any + two lanes suggests the offload model decides. + """ + for symbol in ( + "s_tvd_rk$m_time_steppers_$ck_L486_6", + "s_tvd_rk$m_time_steppers_$ck_L486_16", + "__omp_offloading_8116438_1c00689b__QMm_time_steppersPs_tvd_rk_l486", + ): + summary = summarize_rocm_debug_agent(ROCM_AGENT_FIXTURE_631.replace("s_tvd_rk$m_time_steppers_$ck_L486_6", symbol)) + assert symbol in summary and "486" in summary + + +def test_output_without_an_agent_report_falls_back(): + assert summarize_rocm_debug_agent("Memory access fault by GPU node-4 on address 0x1") == "" + assert summarize_rocm_debug_agent("") == "" + + +def test_a_missing_agent_report_on_a_gpu_fault_is_called_out(): + # The drift above is silent by nature -- raw output where a summary should + # be, and nothing saying why. If the agent is reachable and the failure IS a + # GPU fault, an unrecognised report has to say so. + import inspect + + from mfc.test.test import _handle_case + + src = inspect.getsource(_handle_case) + assert "rocm_debug_agent_path() is not None" in src + assert "format has changed" in src + + +# The other two callers. + + +def test_the_bench_runner_summarizes_rather_than_tailing(tmp_path): + """bench.py ran GPU cases with no fault handling at all. + + Padded past log_tail's 60-line window on purpose: with a short fixture the + tail contains the kernel name and this passes against the old behaviour. + """ + from mfc.bench import bench_failure_report + from mfc.common import log_tail + + log = tmp_path / "case.out" + log.write_text(ROCM_AGENT_FIXTURE_720 + "\n".join(f" v{n}: 0x0" for n in range(200)), encoding="utf-8") + assert "_QMm_time_steppersPs_tvd_rk_l486" not in log_tail(str(log)), "fixture too short to distinguish" + + assert "_QMm_time_steppersPs_tvd_rk_l486" in bench_failure_report(str(log)) + + plain = tmp_path / "plain.out" + plain.write_text("ordinary failure\nsomething went wrong\n", encoding="utf-8") + assert "something went wrong" in bench_failure_report(str(plain)) + + +def test_the_shell_summarizer_reports_absence_by_exit_code(tmp_path): + # The case-optimization script is shell, and needs to know when to fall back. + script = pathlib.Path(__file__).resolve().parents[3] / ".github" / "scripts" / "summarize_gpu_fault.py" + + agent = tmp_path / "agent.log" + agent.write_text(ROCM_AGENT_FIXTURE_720, encoding="utf-8") + found = subprocess.run([sys.executable, str(script), str(agent)], capture_output=True, text=True, check=False) + assert found.returncode == 0 and "_QMm_time_steppersPs_tvd_rk_l486" in found.stdout + + plain = tmp_path / "plain.log" + plain.write_text("ordinary failure\n", encoding="utf-8") + missing = subprocess.run([sys.executable, str(script), str(plain)], capture_output=True, text=True, check=False) + assert missing.returncode == 1 and missing.stdout.strip() == "" + + +def test_restart_cases_carry_the_diagnostics_too(): + # They reach the GPU through run_restart, which took no env at all. + import inspect + + from mfc.test.case import TestCase + + assert "env" in inspect.signature(TestCase.run_restart).parameters diff --git a/toolchain/mfc/test_build_preflight.py b/toolchain/mfc/test_build_preflight.py index ab6d16e03..f510e26be 100644 --- a/toolchain/mfc/test_build_preflight.py +++ b/toolchain/mfc/test_build_preflight.py @@ -122,17 +122,22 @@ def test_a_failing_probe_stops_before_the_solver_build(workspace): assert len(trace.read_text().splitlines()) == 1, "solver build must not be attempted" -def test_a_pypi_failure_records_a_cluster_outage(workspace): +def test_a_pypi_failure_is_an_ordinary_build_failure(workspace): + """A flaky download must not red out the rest of the cluster. + + The dependency install happens before any compute is committed -- on the + login node for Frontier -- so a failed fetch costs no allocation and there + is nothing to protect the matrix from. Recording it as a cluster-wide outage + skipped every other job on that cluster, including ones whose tests had + already passed, and uv already retries internally. + """ tmp_path, install_mfc, run, _ = workspace install_mfc(full_build_stdout=PYPI_FAILURE, full_build_rc=1) - run() - assert outage_recorded(tmp_path) + result = run() -def test_a_pypi_failure_reports_the_outage_exit_code(workspace): - tmp_path, install_mfc, run, _ = workspace - install_mfc(full_build_stdout=PYPI_FAILURE, full_build_rc=1) - assert run().returncode == 78 + assert result.returncode == 1, "a failed download must fail only its own job" + assert not outage_recorded(tmp_path), "a failed download must not trip the cluster breaker" def test_an_ordinary_compile_error_is_not_treated_as_an_outage(workspace): @@ -152,23 +157,6 @@ def test_the_build_output_is_still_shown_when_it_fails(workspace): assert "ftn-2116" in result.stdout + result.stderr -def test_a_pypi_failure_during_the_probe_build_records_an_outage(workspace): - # The probe build is now the first mfc.sh call in the job, so it is what - # bootstraps build/venv from PyPI -- and on Phoenix clean_build has just - # deleted that venv, so it is rebuilt every time. Classifying only the solver - # build leaves the breaker blind to the outage it exists for. - tmp_path, install_mfc, run, _ = workspace - install_mfc(probe_build_stdout=PYPI_FAILURE, probe_build_rc=1) - run() - assert outage_recorded(tmp_path) - - -def test_a_pypi_failure_during_the_probe_build_reports_the_outage_exit_code(workspace): - tmp_path, install_mfc, run, _ = workspace - install_mfc(probe_build_stdout=PYPI_FAILURE, probe_build_rc=1) - assert run().returncode == 78 - - def test_an_ordinary_probe_build_failure_is_not_an_outage(workspace): tmp_path, install_mfc, run, _ = workspace install_mfc(probe_build_stdout="NVFORTRAN-S-0034-Syntax error\n", probe_build_rc=1) diff --git a/toolchain/mfc/test_ci_outage.py b/toolchain/mfc/test_ci_outage.py deleted file mode 100644 index 24ba338b1..000000000 --- a/toolchain/mfc/test_ci_outage.py +++ /dev/null @@ -1,114 +0,0 @@ -"""Unit tests for .github/scripts/ci-outage.sh. - -When an environment-wide outage hits a cluster -- pypi.org unreachable from a -Frontier login node, say -- every queued CI job rediscovers it independently. -On 2026-08-28 that cost 17 Frontier jobs about 33 minutes each to learn the same -fact. The circuit breaker lets the first job that notices record it on the shared -filesystem so later jobs skip immediately instead of queueing SLURM to fail. - -A breaker that cannot reset is worse than none, so the time-to-live behaviour is -pinned here as tightly as the tripping behaviour. -""" - -import os -import subprocess -import time -from pathlib import Path - -import pytest - -SCRIPT = Path(__file__).resolve().parents[2] / ".github" / "scripts" / "ci-outage.sh" - -CLEAR = 0 -TRIPPED = 1 - - -@pytest.fixture -def state_dir(tmp_path): - return tmp_path / "state" - - -def run(state_dir, *args, ttl=None): - env = {**os.environ, "MFC_CI_STATE_DIR": str(state_dir)} - if ttl is not None: - env["MFC_CI_OUTAGE_TTL_SECONDS"] = str(ttl) - return subprocess.run( - ["bash", str(SCRIPT), *args], - capture_output=True, - text=True, - env=env, - check=False, - ) - - -def test_check_reports_clear_when_nothing_has_been_recorded(state_dir): - assert run(state_dir, "check", "phoenix").returncode == CLEAR - - -def test_check_trips_after_an_outage_is_marked(state_dir): - run(state_dir, "mark", "phoenix", "pypi unreachable") - assert run(state_dir, "check", "phoenix").returncode == TRIPPED - - -def test_check_reports_the_recorded_reason_so_the_log_explains_the_skip(state_dir): - run(state_dir, "mark", "phoenix", "pypi unreachable") - result = run(state_dir, "check", "phoenix") - assert "pypi unreachable" in result.stdout + result.stderr - - -def test_an_outage_on_one_cluster_leaves_the_other_alone(state_dir): - run(state_dir, "mark", "frontier", "pypi unreachable") - assert run(state_dir, "check", "phoenix").returncode == CLEAR - - -def test_an_outage_older_than_the_ttl_stops_tripping(state_dir): - run(state_dir, "mark", "phoenix", "pypi unreachable", ttl=1) - time.sleep(2) - assert run(state_dir, "check", "phoenix", ttl=1).returncode == CLEAR - - -def test_clear_resets_the_breaker(state_dir): - run(state_dir, "mark", "phoenix", "pypi unreachable") - run(state_dir, "clear", "phoenix") - assert run(state_dir, "check", "phoenix").returncode == CLEAR - - -def test_marking_works_when_the_state_directory_does_not_exist_yet(state_dir): - # The first job on a fresh runner must not fail just because nothing has - # created the directory. - assert not state_dir.exists() - assert run(state_dir, "mark", "phoenix", "pypi unreachable").returncode == 0 - - -def test_a_corrupt_marker_is_treated_as_clear_rather_than_wedging_ci(state_dir): - # A truncated or garbled marker must never become an un-clearable breaker. - run(state_dir, "mark", "phoenix", "pypi unreachable") - marker = next(state_dir.glob("*phoenix*")) - marker.write_text("not-a-timestamp\n") - assert run(state_dir, "check", "phoenix").returncode == CLEAR - - -def test_check_names_the_marker_file_so_a_human_can_clear_it(state_dir): - run(state_dir, "mark", "phoenix", "pypi unreachable") - result = run(state_dir, "check", "phoenix") - assert str(state_dir) in result.stdout + result.stderr - - -def test_an_unknown_subcommand_fails_with_a_usage_error(state_dir): - # Distinct from both CLEAR and TRIPPED so a typo in a workflow can never be - # silently read as "no outage". - result = run(state_dir, "frobnicate", "phoenix") - assert result.returncode == 2 - assert "usage" in (result.stdout + result.stderr).lower() - - -def test_a_non_numeric_ttl_falls_back_to_the_default_instead_of_erroring(state_dir): - # A typo in MFC_CI_OUTAGE_TTL_SECONDS must not make `check` exit with a code - # that is neither clear nor tripped -- that would gate CI on a config slip. - run(state_dir, "mark", "phoenix", "pypi unreachable") - assert run(state_dir, "check", "phoenix", ttl="not-a-number").returncode == TRIPPED - - -def test_an_empty_ttl_falls_back_to_the_default(state_dir): - run(state_dir, "mark", "phoenix", "pypi unreachable") - assert run(state_dir, "check", "phoenix", ttl="").returncode == TRIPPED diff --git a/toolchain/mfc/test_classify_build_failure.py b/toolchain/mfc/test_classify_build_failure.py deleted file mode 100644 index 19bca303e..000000000 --- a/toolchain/mfc/test_classify_build_failure.py +++ /dev/null @@ -1,73 +0,0 @@ -"""Tests for .github/scripts/classify-build-failure.sh. - -The classifier decides whether a failed build was a cluster-wide dependency -outage. Getting it wrong in either direction is costly: a missed outage means -every matrix job rediscovers it, and a false positive halts CI on an ordinary -compile error. -""" - -import os -import subprocess -from pathlib import Path - -import pytest - -SCRIPT = Path(__file__).resolve().parents[2] / ".github" / "scripts" / "classify-build-failure.sh" - -OUTAGE = 78 -NOT_AN_OUTAGE = 0 - - -@pytest.fixture -def classify(tmp_path): - def _run(log_text): - log = tmp_path / "build.log" - log.write_text(log_text) - return subprocess.run( - ["bash", str(SCRIPT), str(log), "frontier"], - capture_output=True, - text=True, - check=False, - env={**os.environ, "MFC_CI_STATE_DIR": str(tmp_path / "state")}, - ).returncode - - return _run - - -@pytest.mark.parametrize( - "line", - [ - " |-> Failed to fetch: `https://pypi.org/simple/hatch-vcs/`", # uv, backticked - "Failed to fetch: https://pypi.org/simple/hatch-vcs/", # plain, no wrapper - " Failed to fetch: https://pypi.org/simple/build/", # padded - "mfc: ERROR > (venv) Installation failed.", - "mfc: WARNING > (venv) uv install failed; clearing the uv cache", - ], -) -def test_dependency_outages_are_recognised_in_every_formatting_variant(classify, line): - assert classify(line + "\n") == OUTAGE - - -@pytest.mark.parametrize( - "line", - [ - "NVFORTRAN-S-0034-Syntax error at or near end of line", - "ftn-2116 ftn: INTERNAL", - "clang: error: ld.lld command failed with exit code 1", - "CMake Error: could not find HDF5", - "Failed to fetch: https://example.com/not-pypi", - ], -) -def test_ordinary_build_failures_are_not_outages(classify, line): - assert classify(line + "\n") == NOT_AN_OUTAGE - - -def test_an_absent_log_is_never_called_an_outage(tmp_path): - result = subprocess.run( - ["bash", str(SCRIPT), str(tmp_path / "nope.log"), "frontier"], - capture_output=True, - text=True, - check=False, - env={**os.environ, "MFC_CI_STATE_DIR": str(tmp_path / "state")}, - ) - assert result.returncode == NOT_AN_OUTAGE diff --git a/toolchain/mfc/test_frontier_deps.py b/toolchain/mfc/test_frontier_deps.py index 82695c718..d2adaa3a9 100644 --- a/toolchain/mfc/test_frontier_deps.py +++ b/toolchain/mfc/test_frontier_deps.py @@ -1,10 +1,11 @@ """Tests for .github/workflows/frontier/build.sh. Frontier installs its Python dependencies on the login node, in the "Fetch -Dependencies" step, before any SLURM job exists. That is where the PyPI outage -of 2026-08-28 actually landed -- 17 jobs, ~33 minutes each, all learning the -same thing independently. Classifying only the in-allocation build would leave -the breaker blind to the case that motivated it. +Dependencies" step, before any SLURM job exists -- so a failed download costs no +allocation and must fail only its own job. It used to be recorded as a +cluster-wide outage, which then skipped every other job on that cluster. + +`outage_recorded` stays as an assertion that nothing does that any more. """ import os @@ -74,17 +75,22 @@ def test_a_successful_dependency_fetch_records_nothing(workspace): assert not outage_recorded(tmp_path) -def test_a_pypi_outage_on_the_login_node_is_recorded(workspace): +def test_a_pypi_failure_is_an_ordinary_build_failure(workspace): + """A flaky download must not red out the rest of the cluster. + + The dependency install happens before any compute is committed -- on the + login node for Frontier -- so a failed fetch costs no allocation and there + is nothing to protect the matrix from. Recording it as a cluster-wide outage + skipped every other job on that cluster, including ones whose tests had + already passed, and uv already retries internally. + """ tmp_path, install_mfc, run = workspace install_mfc(stdout=PYPI_FAILURE, rc=1) - run() - assert outage_recorded(tmp_path) + result = run() -def test_a_pypi_outage_on_the_login_node_reports_the_outage_exit_code(workspace): - tmp_path, install_mfc, run = workspace - install_mfc(stdout=PYPI_FAILURE, rc=1) - assert run().returncode == 78 + assert result.returncode == 1, "a failed download must fail only its own job" + assert not outage_recorded(tmp_path), "a failed download must not trip the cluster breaker" def test_an_ordinary_dependency_failure_is_not_an_outage(workspace): diff --git a/toolchain/mfc/test_monitor_ci_summary.py b/toolchain/mfc/test_monitor_ci_summary.py new file mode 100644 index 000000000..8f07362cf --- /dev/null +++ b/toolchain/mfc/test_monitor_ci_summary.py @@ -0,0 +1,118 @@ +"""What a failed job puts on the run's summary page. + +Learning why a job failed used to mean opening a log of tens of thousands of +lines, and an infrastructure fault looked exactly like a test failure until you +did. These paths were shipped once with only a syntax check behind them, so +they are exercised here for real. +""" + +import os +import stat +import subprocess +from pathlib import Path + +import pytest + +SCRIPTS = Path(__file__).resolve().parents[2] / ".github" / "scripts" + +GPU_FAULT_OUTPUT = """\ +[ 0%] Time step 1 +=== GPU fault summary (rocm-debug-agent, 14276 lines collapsed) === +Memory access fault by GPU node-5 on address 0x1557da700000. +faulting kernel(s): s_tvd_rk$m_time_steppers_$ck_L486_6 +faulting waves: 125 [MEMORY_VIOLATION x125] +NOTE: waves halt on fault detection. +""" + + +@pytest.fixture +def slurm(tmp_path): + """A finished SLURM job whose exit code and output the test chooses.""" + binz = tmp_path / "bin" + binz.mkdir() + + def configure(exit_code, output): + def exe(name, body): + path = binz / name + path.write_text(body) + path.chmod(path.stat().st_mode | stat.S_IEXEC) + + exe("squeue", "#!/bin/bash\nexit 0\n") + exe("sacct", f'#!/bin/bash\nfor a in "$@"; do [ "$a" = "--format=ExitCode" ] && {{ echo "{exit_code}"; exit 0; }}; done\necho FAILED\n') + exe("scontrol", f'#!/bin/bash\necho "ExitCode={exit_code}"\n') + exe("scancel", "#!/bin/bash\nexit 0\n") + + out = tmp_path / "job.out" + out.write_text(output) + summary = tmp_path / "summary.md" + summary.write_text("") + return out, summary + + return tmp_path, binz, configure + + +def run(tmp_path, binz, out, summary): + return subprocess.run( + ["bash", str(SCRIPTS / "monitor_slurm_job.sh"), "1234", str(out)], + capture_output=True, + text=True, + cwd=tmp_path, + check=False, + timeout=180, + env={ + **os.environ, + "PATH": f"{binz}:{os.environ['PATH']}", + "MFC_MONITOR_POLL_SECONDS": "0", + "MFC_MONITOR_RECHECK_SECONDS": "0", + "GITHUB_STEP_SUMMARY": str(summary), + }, + ) + + +def test_an_infrastructure_fault_names_the_node_and_says_it_is_not_a_test_failure(slurm): + tmp_path, binz, configure = slurm + out, summary = configure("77:0", "some output\nMFC_FAULT_NODE=frontier9999\n") + + assert run(tmp_path, binz, out, summary).returncode == 77 + + text = summary.read_text() + assert "frontier9999" in text + assert "not a code or test failure" in text + + +def test_a_gpu_fault_puts_the_faulting_kernel_on_the_summary_page(slurm): + tmp_path, binz, configure = slurm + out, summary = configure("1:0", GPU_FAULT_OUTPUT) + + assert run(tmp_path, binz, out, summary).returncode == 1 + + text = summary.read_text() + assert "s_tvd_rk$m_time_steppers_$ck_L486_6" in text, "the whole point is naming the kernel" + assert "GPU memory fault" in text + + +def test_a_successful_job_writes_nothing(slurm): + tmp_path, binz, configure = slurm + out, summary = configure("0:0", "all good\n") + + assert run(tmp_path, binz, out, summary).returncode == 0 + assert summary.read_text() == "" + + +def test_the_job_output_is_not_printed_twice(slurm): + """It used to be: streamed with tail -f, then cat in full at the end. + + Measured at three copies of every line on a GPU job, and 65,000 lines of + offload diagnostics repeated for a single fault. + """ + tmp_path, binz, configure = slurm + lines = 400 + out, summary = configure("1:0", "\n".join(f"job output line {n}" for n in range(lines)) + "\n") + + result = run(tmp_path, binz, out, summary) + + # Volume, not a marker: `tail -f` on a file that is already complete shows + # only its last lines, whereas in CI it follows the file as it is written. + # What must hold either way is that the whole file is not emitted again -- + # before the fix, stdout carried a full copy of it. + assert result.stdout.count("job output line ") < lines, "the whole log was reprinted" diff --git a/toolchain/mfc/test_monitor_exit_codes.py b/toolchain/mfc/test_monitor_exit_codes.py index 2b87016e8..59347905b 100644 --- a/toolchain/mfc/test_monitor_exit_codes.py +++ b/toolchain/mfc/test_monitor_exit_codes.py @@ -1,9 +1,9 @@ -"""The infrastructure exit codes must survive the trip back to the submit wrapper. +"""The infrastructure exit code must survive the trip back to the submit wrapper. -preflight.sh exits 77 (node fault) or 78 (recorded outage) inside the SLURM job. +preflight.sh exits 77 (node fault) inside the SLURM job. That becomes the job's ExitCode, which monitor_slurm_job.sh reads and run_monitored_slurm_job.sh relays to the resubmit loop. If either layer flattens -them to 1 -- as both did for every non-zero code before -- the loop sees a +it to 1 -- as both did for every non-zero code before -- the loop sees a generic failure and the node is never excluded. """ @@ -60,7 +60,7 @@ def run_script(tmp_path, binz, name, *args): ) -@pytest.mark.parametrize("job_exit,expected", [("77:0", 77), ("78:0", 78)]) +@pytest.mark.parametrize("job_exit,expected", [("77:0", 77)]) def test_monitor_relays_the_infrastructure_exit_code(slurm, job_exit, expected): tmp_path, binz, configure = slurm out = configure(job_exit) @@ -73,7 +73,7 @@ def test_monitor_still_reports_an_ordinary_failure_as_one(slurm): assert run_script(tmp_path, binz, "monitor_slurm_job.sh", "1234", str(out)).returncode == 1 -@pytest.mark.parametrize("monitor_exit", [77, 78]) +@pytest.mark.parametrize("monitor_exit", [77]) def test_the_runner_relays_the_infrastructure_exit_code(tmp_path, monitor_exit): # Stub the inner monitor so this exercises only the relaying layer. scripts = tmp_path / "scripts" diff --git a/toolchain/mfc/test_preflight.py b/toolchain/mfc/test_preflight.py index d376ca838..9125fb667 100644 --- a/toolchain/mfc/test_preflight.py +++ b/toolchain/mfc/test_preflight.py @@ -56,7 +56,9 @@ def workspace(tmp_path): def install_launcher(workspace, name): """A passthrough launcher that records its argv, mirroring mpirun/srun.""" launcher = workspace / "bin" / name - launcher.write_text("#!/bin/bash\n" f'echo "$@" >> {workspace}/launched.txt\n' 'while [ "${1:0:1}" = "-" ]; do shift; case "$1" in [0-9]*) shift;; esac; done\n' 'exec "$@"\n') + # Skips options and their values by looking for the first executable + # argument, so it does not need to know which flags take a value. + launcher.write_text("#!/bin/bash\n" f'echo "$@" >> {workspace}/launched.txt\n' 'while [ $# -gt 0 ] && [ ! -x "$1" ]; do shift; done\n' 'exec "$@"\n') launcher.chmod(launcher.stat().st_mode | stat.S_IEXEC) return launcher @@ -129,17 +131,6 @@ def test_passes_when_no_syscheck_binary_was_built(workspace): assert run(workspace).returncode == HEALTHY -def test_skips_when_the_cluster_is_already_known_to_be_down(workspace): - write_syscheck(workspace, 0) - subprocess.run( - ["bash", str(SCRIPTS / "ci-outage.sh"), "mark", "phoenix", "pypi unreachable"], - env={**os.environ, "MFC_CI_STATE_DIR": str(workspace / "state")}, - capture_output=True, - check=True, - ) - assert run(workspace).returncode == OUTAGE - - def test_does_not_report_a_node_fault_merely_because_pmix_printed_a_warning(workspace): # PMIX_ERR_NO_PERMISSIONS in dstore_base.c is benign noise: it appears in # 16% of passing self-hosted jobs and only 9% of failing ones. Gating on it @@ -173,17 +164,6 @@ def test_a_missing_launcher_is_not_blamed_on_the_node(workspace): assert run(workspace, "frontier", "gpu").returncode == HEALTHY -def test_a_breaker_that_cannot_be_read_does_not_halt_the_job(workspace): - # Exit 1 from ci-outage.sh means "tripped"; any other failure means the check - # itself broke. Conflating them turns a bug in the breaker into a CI outage. - write_syscheck(workspace, 0) - (workspace / "state").chmod(0o000) - try: - assert run(workspace, "phoenix", "gpu").returncode == HEALTHY - finally: - (workspace / "state").chmod(0o755) - - def test_it_refuses_to_judge_a_node_outside_a_slurm_allocation(workspace): # mfc.sh load is used for building on login nodes too (bench.yml and # frontier/build.sh both load the GPU module set there). A probe that ran in diff --git a/toolchain/mfc/test_preflight_launcher.py b/toolchain/mfc/test_preflight_launcher.py new file mode 100644 index 000000000..42b36d57f --- /dev/null +++ b/toolchain/mfc/test_preflight_launcher.py @@ -0,0 +1,105 @@ +"""The preflight must judge the node, not the launcher. + +Open MPI's default binding fails on some Phoenix nodes before the binary is +launched at all ("hwloc_set_cpubind returned Error"). Read as a node fault that +excluded three healthy nodes and failed the run two jobs deep. +""" + +import os +import stat +import subprocess +from pathlib import Path + +SCRIPTS = Path(__file__).resolve().parents[2] / ".github" / "scripts" + +LAUNCH_FAILURE = """\ +-------------------------------------------------------------------------- +Open MPI tried to bind a new process, but something went wrong. The +process was killed without launching the target application. + Error message: hwloc_set_cpubind returned "Error" for bitmap "0" +-------------------------------------------------------------------------- +mpirun was unable to start the specified application as it encountered an error: +Error name: The specified application failed to start +""" + + +def _fake_cluster(tmp_path, mpirun_body): + """A tree with a syscheck binary and a stubbed mpirun.""" + binz = tmp_path / "bin" + binz.mkdir() + mpirun = binz / "mpirun" + mpirun.write_text(mpirun_body) + mpirun.chmod(mpirun.stat().st_mode | stat.S_IEXEC) + + install = tmp_path / "build" / "install" / "gpu-acc-test" / "bin" + install.mkdir(parents=True) + (install / "syscheck").write_text("#!/bin/bash\nexit 1\n") + (install / "syscheck").chmod(0o755) + return binz + + +def _run(tmp_path, binz): + return subprocess.run( + ["bash", str(SCRIPTS / "preflight.sh"), "phoenix", "gpu"], + capture_output=True, + text=True, + cwd=tmp_path, + check=False, + timeout=120, + env={**os.environ, "PATH": f"{binz}:{os.environ['PATH']}", "SLURM_JOB_ID": "1", "SLURMD_NODENAME": "n1"}, + ) + + +def test_a_launch_failure_does_not_condemn_the_node(tmp_path): + binz = _fake_cluster(tmp_path, f"#!/bin/bash\ncat <<'EOF'\n{LAUNCH_FAILURE}EOF\nexit 1\n") + result = _run(tmp_path, binz) + assert result.returncode == 0, f"excluded a node for a launcher failure:\n{result.stdout}" + assert "not evidence about the node" in result.stdout + + +def test_a_binary_that_runs_and_fails_still_condemns_the_node(tmp_path): + binz = _fake_cluster(tmp_path, '#!/bin/bash\nshift $((OPTIND)); echo "GPU init failed"; exit 1\n') + result = _run(tmp_path, binz) + assert result.returncode == 77, f"a real node fault must still be caught:\n{result.stdout}" + + +def test_the_probe_does_not_ask_open_mpi_to_bind(tmp_path): + binz = _fake_cluster(tmp_path, '#!/bin/bash\necho "ARGS: $*"; exit 0\n') + result = _run(tmp_path, binz) + assert "--bind-to none" in result.stdout, "a single-rank probe must not rely on default binding" + + +def test_a_launcher_that_rejects_our_options_still_probes(tmp_path): + """Otherwise the preflight switches itself off silently. + + A failed launch is treated as inconclusive, so a launcher that rejects an + option we pass would fail every probe and quietly stop the preflight ever + catching a real node fault -- worse than failing loudly. + """ + binz = _fake_cluster( + tmp_path, + "#!/bin/bash\n" 'if [ "$1" = "--bind-to" ]; then echo "mpirun: unrecognized option \'--bind-to\'"; exit 1; fi\n' 'echo "RETRIED-WITHOUT-FLAGS"; exit 1\n', + ) + result = _run(tmp_path, binz) + + assert "rejected the probe's options" in result.stdout + assert "RETRIED-WITHOUT-FLAGS" in result.stdout + + +def test_a_login_node_is_never_judged(tmp_path): + """`mfc.sh load` is also used for building on login nodes, where there is no + GPU to probe and srun may not even be permitted.""" + binz = _fake_cluster(tmp_path, "#!/bin/bash\nexit 1\n") + env = {**os.environ, "PATH": f"{binz}:{os.environ['PATH']}"} + env.pop("SLURM_JOB_ID", None) + result = subprocess.run( + ["bash", str(SCRIPTS / "preflight.sh"), "phoenix", "gpu"], + capture_output=True, + text=True, + cwd=tmp_path, + check=False, + timeout=120, + env=env, + ) + assert result.returncode == 0 + assert "not inside a SLURM allocation" in result.stdout diff --git a/toolchain/mfc/test_submit_requeue.py b/toolchain/mfc/test_submit_requeue.py index 4fe0ceb63..d75b3c2b3 100644 --- a/toolchain/mfc/test_submit_requeue.py +++ b/toolchain/mfc/test_submit_requeue.py @@ -140,22 +140,6 @@ def test_a_known_outage_is_not_resubmitted(rig): assert len(result.submissions) == 1 -def test_no_job_is_submitted_while_the_cluster_is_under_a_recorded_outage(rig): - # ci-outage.sh's whole promise is that later jobs "exit immediately instead - # of submitting a SLURM job that is going to fail". Checking it only inside - # the allocation means every job still pays the queue wait first -- hours on - # Phoenix embers -- before discovering the marker. - subprocess.run( - ["bash", str(rig.scripts / "ci-outage.sh"), "mark", "phoenix", "pypi unreachable"], - env={**os.environ, "MFC_CI_STATE_DIR": str(rig.state_dir)}, - capture_output=True, - check=True, - ) - result = rig("0") - assert len(result.submissions) == 0 - assert result.returncode == 78 - - def test_the_default_bound_is_one_requeue(rig): """One requeue, not two.