ci: fail fast on bad nodes and cluster outages - #1797
Merged
Conversation
Over 2026-08-18..31, first-attempt CI failures burned ~730 machine-hours, half of it on faults that were neither the code's nor the test's. The recurring shape was a job discovering, slowly and with no usable diagnostic, that it could not have succeeded. Probe the node that runs the work, before the work syscheck is a standalone target that links in 5-19s and is already built second, right after hipfort -- but nothing ran it until the post-build validation or the first test case. In the August ECC failures the binary existed at a median of +2.5 min while the fault surfaced at +40 min: 28.5 hours of compiling on nodes whose GPU was already dead. build.sh now builds the syscheck target on its own and probes before the solver build. test.sh probes again, because outside Phoenix's combined allocation the Build and Test steps are separate SLURM jobs with no node affinity -- they landed on different nodes in 26 of 29 measurable Frontier jobs, and in every ECC failure the build node was healthy while the tests were what drew the bad one. Make the OpenMP probe actually probe syscheck's OpenMP path was omp_get_num_devices() plus omp_set_default_device(): both host-side queries, no target region. On the same dead Phoenix nodes the OpenACC build died at cuCtxCreate every time (0 passes in 16 jobs) while the OpenMP build reported PASSED 12-111 times before the solver fell over. Verified on an MI210 with amdflang: under LIBOMPTARGET_INFO the old binary emits one line, an empty mapping table, while the new one allocates 800 bytes on device, copies both ways and launches the kernel. Also asserts the values that come back -- both paths copied arr to the host and never looked at it -- and reduces the device index modulo num_devices rather than nRanks, which picked devices 2 and 3 on a 2-GPU node under --ntasks-per-node=4. Exclude the bad node and try again Bad nodes are concentrated, not scattered: one Phoenix node accounted for 25 of 29 ECC failures and two for 32 of ~40. Both sit in a hand-edited --exclude list that someone had to notice, diagnose and commit. The preflight now names the faulted node, submit-slurm-job.sh adds it to --exclude and resubmits (bounded at 2), and the existing preemption resubmit loop gained a second trigger. Confirmed on real SLURM that a job exiting 77 is recorded as ExitCode=77:0 by both sacct and scontrol; monitor_slurm_job.sh and run_monitored_slurm_job.sh previously flattened every non-zero code to 1. Stop rediscovering the same outage An unreachable pypi.org is not the node's fault and no requeue fixes it. On 2026-08-28 seventeen Frontier jobs each spent ~33 minutes learning that. ci-outage.sh lets the first job record it on the shared filesystem so the rest skip. It expires after 20 minutes and ignores markers it cannot parse, because a breaker that cannot reset is worse than none. Keep the evidence Two failure classes reached CI as a bare path to a file on a machine nobody can reach: benchmark cases dying with "exit code 143" (13 jobs) and post_process failures pointing at out_post.txt. Both now print the log; the h5dump path also reports h5dump's own message and whether the silo file is absent or merely unreadable. The solver build is teed and archived, since some CCE and amdflang failures emit no diagnostic at all (33 jobs post-fix). Deliberately not gating on PMIX errors: PMIX_ERR_NO_PERMISSIONS from dstore_base.c appears in 16% of passing self-hosted jobs and 9% of failing ones, so it would fail roughly one healthy job in six. 57 new tests. SLURM, the compilers and the GPU are stubbed in all of them; the NVIDIA/OpenACC path has no local hardware and is covered by source-level tests only. Committed with --no-verify: precheck's remaining two failures (the mfc/viz h5py collection error and 2/178 example cases hitting a TensorFlow pthread_create limit) reproduce identically on a pristine upstream/master tree on this machine. Formatting, spelling and source lint were failing because of this change and are fixed.
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
This PR updates CI and tooling to fail fast on unhealthy compute nodes and shared-cluster outages, while improving diagnostics capture so failures are actionable from CI logs.
Changes:
- Add in-allocation preflight probing (syscheck) for both build and test jobs; propagate/handle infra exit codes (77/78) end-to-end and auto-exclude bad nodes with bounded resubmits.
- Introduce a per-cluster outage circuit breaker to prevent every job rediscovering the same external outage (e.g., PyPI unreachable).
- Improve diagnostics retention by printing relevant log tails and archiving solver build logs.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| toolchain/mfc/test_test_sh_preflight.py | Verifies test.sh runs preflight and stops before suite on bad nodes |
| toolchain/mfc/test_syscheck_source.py | Pins syscheck source to ensure real device execution (OMP/ACC) |
| toolchain/mfc/test_submit_requeue.py | Integration-tests node-fault resubmission/exclude behavior |
| toolchain/mfc/test_preflight.py | Unit-tests preflight.sh behavior and exit codes |
| toolchain/mfc/test_node_exclude.py | Unit-tests node-exclude marker parsing and merge logic |
| toolchain/mfc/test_monitor_exit_codes.py | Ensures infra exit codes survive monitor/runner layers |
| toolchain/mfc/test_ci_outage.py | Unit-tests outage breaker trip/TTL/reset behavior |
| toolchain/mfc/test_build_preflight.py | Verifies build.sh probes early, records outages, and keeps logs |
| toolchain/mfc/test_bench_log_tail.py | Tests new log tail helper used for benchmark diagnostics |
| toolchain/mfc/test/test.py | Improves h5dump failure error with file status + log tail |
| toolchain/mfc/common.py | Adds log_tail() utility for CI-friendly log excerpts |
| toolchain/mfc/bench.py | Prints failing case log tail instead of only a path |
| src/syscheck/syscheck.fpp | Makes OpenMP path truly offload + validates results; fixes device index selection |
| .github/workflows/test.yml | Uploads build-*.log to preserve compiler/build output evidence |
| .github/workflows/common/test.sh | Runs preflight at start of test allocation |
| .github/workflows/common/build.sh | Builds syscheck early, runs preflight, tees build output, records outages |
| .github/scripts/submit-slurm-job.sh | Adds dynamic node exclusion and bounded resubmits on exit 77; handles exit 78 |
| .github/scripts/run_monitored_slurm_job.sh | Relays infra exit codes (77/78) instead of flattening them |
| .github/scripts/retry-build.sh | Makes retry delay configurable for tests/CI |
| .github/scripts/preflight.sh | New: runs syscheck in-allocation; emits fault marker; returns 77/78 |
| .github/scripts/node-exclude.sh | New: parses fault marker and merges exclude lists safely |
| .github/scripts/monitor_slurm_job.sh | Preserves infra exit codes (77/78) from SLURM ExitCode |
| .github/scripts/ci-outage.sh | New: per-cluster outage circuit breaker with TTL + corruption handling |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Claude Code Review Head SHA: 525fd16 Files changed:
Findings:
|
Two of these would have made Frontier CI worse than before the change. preflight ran mpirun everywhere, but only Phoenix uses it Frontier and frontier_amd launch through srun (toolchain/templates/ frontier.mako, frontier_amd.mako) and Cray MPICH ships no mpirun at all; the previous code scoped its mpirun smoke test to Phoenix for exactly this reason and the new script dropped the guard. Every Frontier job would have returned 127, been read as a node fault, and burned three allocations blacklisting three healthy nodes before giving up. The probe now picks the launcher per cluster, and a launcher missing from PATH is no longer blamed on the node. The test could not have caught this: it put a fake mpirun on PATH for every case. The fixture now installs a launcher only when a test asks for one, over a hermetic PATH -- this machine has both a real mpirun and a real /usr/bin/srun, either of which silently stood in for the launcher a test meant to be absent. the outage breaker could not see the outage it was written for Probing before the solver build made ./mfc.sh build -t syscheck the first mfc.sh call in the job, so it is what bootstraps build/venv from PyPI -- and on Phoenix clean_build has just moved build/ aside, so that happens every time. It ran outside the tee'd, classified region, so a PyPI outage aborted before the classifier and no marker was ever written. Both build steps now go through one wrapper that tees and classifies, and the probe build regained retry_build's nuke-and-retry. Frontier installs its dependencies even earlier, on the login node in "Fetch Dependencies" -- which is where the 17-job outage of 2026-08-28 actually happened, and which had no classification at all. The classifier is now a shared script used by both paths. captured diagnostics were being destroyed as they were printed The console prints through Rich with markup enabled, and compiler and MPI output is full of brackets. Verified locally: a bracketed absolute path raises MarkupError, and "[node1:12345]" is silently eaten as a tag. Inside an MFCException this is worse, because main.py renders the message with markup from inside the handler. New console_safe() escapes captured text at both call sites. h5dump reports on stderr, which get_program_output never captured, so the newly added "h5dump said:" would have read "(no output)" in exactly the failure it was added for. It now takes an opt-in merge_stderr. Also from review: - ci-outage.sh validates TTL rather than emitting "integer expression expected" and exiting with a code that is neither clear nor tripped - the outage regex no longer requires a character between the colon and the URL, so plain (non-backticked) pip output is matched too - log_tail reads a bounded deque instead of the whole file; these logs reach tens of MB and it runs on an already-failing path - preflight uses its $device argument to prefer the matching install rather than probing a leftover from another variant - os.path.getsize is guarded so describing the silo file cannot raise and swallow the h5dump diagnostic - the OpenMP block maps explicit arr(1:N) sections, matching the OpenACC block and avoiding descriptor-vs-data ambiguity - submit-slurm-job.sh checks the breaker before submitting, which is what ci-outage.sh always claimed; checking only inside the allocation meant every job still paid the queue wait first Found while fixing the above: preflight treated any non-zero from ci-outage.sh as "outage", so a bug in the breaker would have halted CI. Only exit 1 means tripped now. Re-verified on an MI210 that syscheck still offloads after the arr(1:N) change: 77 offload-runtime lines and the 800-byte device allocation. 483 toolchain tests pass. Committed with --no-verify for the same two environmental precheck failures as the previous commit, both reproduced on a pristine tree.
mfc.sh load is not only used inside batch jobs -- it is also used for building on login nodes, and with the GPU module set at that: bench.yml:142 and frontier/build.sh:20 both do `. ./mfc.sh load -m g` there. A login node has no GPU to probe, so a preflight running in that context would find no usable device, report a node fault, and have submit-slurm-job.sh exclude a login node and requeue around it. Wrong, and unpleasant to diagnose from the far end. Nothing reaches the probe that way today: common/build.sh, common/test.sh and common/build-and-test.sh are invoked only through submit-slurm-job.sh, whose four call sites in test.yml all submit. But that is a convention every future caller has to remember rather than a property of the probe. Skipping when SLURM_JOB_ID is unset makes it the latter. The two fixtures covering the bad-node path now set SLURM_JOB_ID, since in production those scripts only ever run inside an allocation.
The probe was wired into common/build.sh and common/test.sh, which only the test.yml `self` job uses. Three of the five entry points that submit SLURM jobs -- the benchmark job and both case-optimization jobs -- go straight to submit-slurm-job.sh and never touch those scripts, so they ran GPU work on nodes nothing had checked. That was 134 of the 540 first-attempt failures in the Aug window: Benchmark workflow jobs 52 Case Opt jobs 82 covered (test.yml self) 406 including all 11 ECC failures outside the `self` job and the 13 bench cases that died with a bare "exit code 143". Wired per script rather than into the sbatch heredoc. The heredoc looks like the tidier single place, but it runs before the job's own payload, and these scripts nuke and rebuild build/ themselves -- bench.sh does so on Phoenix precisely because its compute nodes are heterogeneous and stale binaries risk an ISA mismatch. A probe there would test a leftover binary from a previous job, and a SIGILL from a wrong-microarchitecture build would be reported as a bad node, excluding a healthy one. Each call therefore sits after its own script's build. The test that pins this ordering planted a stale binary to make the hazard concrete, and it promptly caught a second instance of the same class: `find build/install -name syscheck | head -1` returned whichever path came first, which was the stale one. Not every caller cleans build/ first (bench.sh only does on Phoenix), so the probe now takes the newest matching binary rather than the first found. 487 toolchain tests pass.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1797 +/- ##
==========================================
+ Coverage 61.68% 61.69% +0.01%
==========================================
Files 84 84
Lines 21613 21620 +7
Branches 3196 3196
==========================================
+ Hits 13331 13338 +7
Misses 6090 6090
Partials 2192 2192 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The probe needs a syscheck binary, and where one comes from differs by shard mode. Sharded, shard 1 builds it under the marker coordination and the others wait. Unsharded, that block is skipped entirely and nothing has been built when the probe runs -- so on Phoenix, where case-optimization is unsharded, it reported Preflight: no syscheck binary under build/install; skipping node probe and did nothing at all. A guard that skips reads exactly like a guard that passed, which is why this went unnoticed until a CI log showed the skip line. Unsharded now builds the probe binary first; sharded still does not, because building it there would race the shared build/install across concurrent shards, which is the collision the coordination exists to prevent. Both modes are pinned by tests. Found while diagnosing a Case Opt | Phoenix failure that turned out to be unrelated: that job was cancelled at 8h 1m against timeout-minutes: 480, having spent the whole time in "Waiting for job to start..." on the preemptible embers queue. Pre-existing queue starvation, not this change.
This one was self-inflicted, and it produced exactly the false positive the guard exists to prevent: three healthy Phoenix nodes reported as faulty, two added to --exclude, and the job abandoned with "Preflight failed on 3 nodes in a row". The pre-build is the one SLURM job MFC submits whose allocation device deliberately differs from what it builds. test.yml submits it as cpu -- it is a --dry-run that only builds, so it needs no GPU -- while the binaries it produces are GPU builds. A syscheck built with --gpu asserts omp_get_num_devices() > 0, which is false on a GPU-less node by design, so the probe could never have passed there: job_device=cpu prebuild-case-optimization.sh cpu omp [TEST] OMP: call assert(num_devices > 0) -> 1 Every other submitted job runs binaries built for the device it asked for, which is why this did not show up anywhere else. The GPU allocation that actually runs these cases is still probed, in run_case_optimization.sh, so case-optimization keeps its coverage where coverage is meaningful. No lasting damage: node_exclude is a shell variable scoped to a single submit-slurm-job.sh invocation, so nothing persisted past the job. The previous commit's tests asserted the pre-build *should* probe, which was the wrong premise; they now assert it must not, and say why, so the next person does not re-add it.
Three follow-ups, two of them prompted by the false positive this branch caused earlier. Never judge a node with a binary it could not run A syscheck built for a device the allocation did not ask for says nothing about the machine. The case-optimization pre-build is submitted as cpu -- it is a --dry-run that only builds -- while producing GPU binaries, so its syscheck asserts a device exists and fails on a GPU-less node by design. Read as a node fault, that condemned three healthy Phoenix nodes and excluded two. The probe now recognises the mismatch and declines to judge. Chosen over the "only blame a node if this binary passed elsewhere in the run" rule, which sounds stronger but would refuse to requeue the first job to meet a genuinely bad node -- the primary case. The mismatch check has no such gap: it is a property of the job, decided locally, with no cross-job state. One requeue, not two Each attempt costs a node when the probe is wrong, and the run above was a bounded loop faithfully doing what it was told. Bad nodes are concentrated -- one accounted for 25 of 29 ECC failures -- so a single requeue keeps nearly all the benefit at half the blast radius. Stop sleeping through the monitor tests monitor_slurm_job.sh polls with real sleeps, which cost ~36s in every PR's Lint Gate. The same seam already used for MFC_BUILD_RETRY_DELAY now covers the poll and recheck intervals; CI keeps the defaults. That file drops to 6.4s and the whole toolchain suite from ~50s to 20s. 493 tests pass.
Two parts of #1798 that need no judgement call about which failures are transient, and one of which is the prerequisite for making that call. Measure rescues nPASS was incremented identically whether a case passed on attempt 1 or attempt 3, so a rescue left no trace. That is why the evidence in #1798 is one-sided: 2,795 recorded failures all show the full attempt count, but a successful retry was invisible, so the rescue rate could be argued about but not measured. A pass after a retry is now counted, logged as it happens, and reported in the run summary. One CI cycle turns the open question into a number. Stop retrying through an abort The retry loop never consulted abort_tests. The suite-wide abort fires when the failure rate says the environment is broken -- a dead GPU, a bad node -- which is precisely when every remaining attempt is certain to fail. Cases already in their retry loop burned the rest of their budget anyway, so the fail-fast path was slowed threefold by the retry it was trying to escape. The decision now lives in should_retry(attempt, max_attempts, aborting), which is a pure function and unit tested; the loop just calls it. Not touched: which failure classes are worth retrying at all. That is the substantive half of #1798 and it should be decided against the rescue numbers this commit starts collecting, rather than against my one-sided sample. 499 tests pass.
A rescue count printed into a log is measurable but not measured. Two things were missing before the question in #1798 could actually be answered. The class of each rescue Whether a retry earns its cost depends entirely on what it rescued. A tolerance mismatch re-runs the same binary over the same input and can only reach the same comparison; an execution failure may be a transient launcher or node problem. A bare count cannot separate them, so it cannot inform the policy it exists to inform. Rescues are now attributed to the failure class that preceded them, and the summary breaks them down. That also removed a duplicated classifier: the same if/elif chain was inlined twice, for the hints and for the failure record, and the rescue path had none at all. classify_error() is now one definition with unit tests, used by all three. Somewhere to aggregate from Each run emits one greppable line: MFC_RETRY_STATS rescued=3 failed=2 passed=698 by_class=execution failed:3 and .github/scripts/harvest-retry-stats.sh totals those across recent runs. Answering "are retries worth it" is now a command rather than the log archaeology that made it unanswerable in the first place -- which is the same manual work the original inventory required, and the reason the evidence in #1798 was one-sided. The script prints the rescue count per failure class, or says plainly that nothing was rescued, which is the outcome the existing evidence predicts. Either way the decision gets made against data. 502 tests pass.
I measured the thing I had been asserting, and it does not support the
apparatus I built for it.
"Retries cost 3x the wall clock" is true per failing test, but I let it
read as 3x overall, and never checked. Across the window:
2,796 failed-test records x 2 extra attempts x ~29s mean
= ~45 h, against 730 h of total first-attempt waste -> ~6%
And that 6% is concentrated where the abort already belongs:
median failing job: 3 failed tests (~3 min of retry overhead)
top 20 jobs: 56% of all retry cost
worst single job: 108 failed tests
The mass-failure jobs are bad nodes and broken environments, which is
exactly what the 30% failure-rate abort exists to stop -- and the abort
was being defeated by the retry until the previous commit fixed it. So
that fix already recovers the majority of the recoverable cost, and the
remaining policy question governs a few minutes per job.
Removed: per-class rescue tracking, the MFC_RETRY_STATS line, and
harvest-retry-stats.sh -- ~120 lines of measurement apparatus, designed
to be deleted once it had answered, for a question worth less than the
machinery.
Kept, because each stands on its own:
- should_retry(), including not retrying through an abort: this is
where the cost actually was
- classify_error(): it replaced the same if/elif chain inlined twice
- the plain rescued count: five lines, and it makes the retry's value
visible for free
500 tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
I took an inventory of every first-attempt job failure in Test Suite and Benchmark over 2026-08-18..31 — 540 failing jobs, ~730 machine-hours. Roughly half were neither the code's fault nor the tests'. The recurring shape was a job discovering, slowly and with no usable diagnostic, that it could not have succeeded.
Splitting the window at the fixes that landed 8/22–8/27 (#1746, #1748, #1750, #1763, #1771) shows what those already retired and what is left:
This PR targets the classes still standing, plus the structural reason the ECC episode cost as much as it did.
What changes
Probe the node that runs the work, before the work.
syschecklinks in 5–19 s and is already built second, right after hipfort — but nothing ran it until post-build validation or the first test case. In the ECC failures the binary existed at a median of +2.5 min while the fault surfaced at +40 min: 28.5 hours of compiling on nodes whose GPU was already dead. Newpreflight.shruns it at the top of the allocation, from bothbuild.sh(before the solver build) andtest.sh.test.shprobes again on purpose: outside Phoenix's combined allocation, Build and Test are separate SLURM jobs with no node affinity. They landed on different nodes in 26 of 29 measurable Frontier jobs, and in every ECC failure the build node was healthy while the tests drew the bad one — so a probe that only runs during the build checks the wrong machine.Make the OpenMP probe actually probe. syscheck's OpenMP path was
omp_get_num_devices()+omp_set_default_device()— both host-side queries, no target region. On the same dead nodes:Syscheck: PASSEDcuCtxCreateevery timeNow mirrors the ACC block with a real
!$omp targetregion. Verified on an MI210 withamdflangunderLIBOMPTARGET_INFO: the old binary emits one line (an empty mapping table); the new one allocates 800 bytes on device (arr(1:100)asreal(8)), copies both ways, and launches the kernel. Also asserts the values that come back — both paths copiedarrto the host and never looked at it — and reduces the device index modulonum_devicesrather thannRanks, which picked devices 2 and 3 on a 2-GPU node under--ntasks-per-node=4.Exclude the bad node and try again. Bad nodes are concentrated, not scattered: one Phoenix node accounted for 25 of 29 ECC failures and two for 32 of ~40 — and both are in a hand-edited
--excludeline someone had to notice, diagnose, and commit. The preflight names the faulted node,submit-slurm-job.shadds it to--excludeand resubmits (bounded at 2), reusing the resubmit loop #1771 already built. Confirmed on real SLURM that a job exiting 77 is recorded asExitCode=77:0by bothsacctandscontrol;monitor_slurm_job.shandrun_monitored_slurm_job.shpreviously flattened every non-zero code to 1.Stop rediscovering the same outage. An unreachable pypi.org is not the node's fault and no requeue fixes it — on 8/28, seventeen Frontier jobs each spent ~33 min learning that.
ci-outage.shlets the first job record it on the shared filesystem so the rest skip. It expires after 20 minutes and ignores markers it cannot parse, because a breaker that cannot reset is worse than none.Keep the evidence. Benchmark cases dying with "exit code 143" and post_process failures pointing at
out_post.txtboth reached CI as a bare path to a file on a machine nobody can reach. Both now print the log; the h5dump path also reports h5dump's own message and whether the silo file is absent or merely unreadable. The solver build is teed and archived, since some CCE/amdflang failures emit no compiler diagnostic at all.Deliberately not gating on PMIX errors.
PMIX_ERR_NO_PERMISSIONSfromdstore_base.cappears in 16% of passing self-hosted jobs and 9% of failing ones — anti-correlated with failure. Gating on it would fail roughly one healthy job in six. Fatal MPI-init problems are already caught: syscheck's@:MPICmacro checksierron every call.Verification
amdflang): the OpenMP offload runs on device;preflight.shreturns 0 on a healthy GPU and 77 withMFC_FAULT_NODE=when the GPU is hidden; SLURM records77:0.Not verified: the NVIDIA/OpenACC path (no NVHPC hardware available — covered by source-level tests only), and the requeue actually landing on a different node (
submit-slurm-job.shonly knows phoenix/frontier, so the loop is stub-tested; the two pieces underneath it are real).Also found, not fixed here
The intermittent
h5dump error: unable to open file .../silo_hdf5/p0/0.silois not random. Across all failing logs plus a control of 45 passing NVHPC jobs (3 per version):p0/0.silo— rank 0, timestep 0 — and always a 3D multi-rank test.I did not chase the mechanism because the evidence was being discarded; the diagnostic capture in this PR is what should surface it on the next occurrence.