Let the benchmark signal go red without holding the deploy back - #1008
Merged
Conversation
epompeii
force-pushed
the
u/ep/benchmarks-non-gating
branch
from
August 23, 2026 16:59
ef4b427 to
4db3e25
Compare
epompeii
marked this pull request as ready for review
August 23, 2026 17:00
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.
Problem
On the devel push run 32651179276,
Test / Track Benchmarks / Track Benchmarks with Bencherfailed on benchmark alerts (an accepted latency regression). That one failure cascaded all the way to the deploy:bencher run ... --error-on-alertexits non-zero, so thetrack_benchmarksjob in.github/workflows/track_benchmarks.ymlfails.Track Benchmarksworkflow concludesfailure.test.ymlcalls it as thetrack_benchmarksjob, so that caller job's result isfailure, and the reusableTestworkflow concludesfailure.ci.yml,deployisneeds: [lint, cli, test, build]withif: ${{ !failure() && !cancelled() && ... }}. Withneeds.test.result == 'failure',!failure()is false andDeployis skipped.ci-successthen readsneeds.test.result != 'success'and exits 1, soCI Successis red too.A benchmark alert is a signal about performance. It is not a statement about whether the build is deployable.
The change
Benchmark alerts inform, they never gate. One line,
continue-on-error: true, on thetrack_benchmarksjob in.github/workflows/track_benchmarks.yml.Why there and not at the call site
continue-on-erroris not among the keywords GitHub accepts on a job that calls a reusable workflow (jobs.<id>.usessupportsname,uses,with,secrets,needs,if,permissions,strategy, andconcurrency). The flag has to live on a real job inside the callee.This is exactly how
Lint / Cargo Deny AdvisoriesandLint / Zizmor Onlinealready work: both carrycontinue-on-error: trueat job level inside the reusablelint.yml, never at theci.ymlcall site, andci-successreadsneeds.lint.resultand stays green when either of them fails. The same conclusion-laundering applies here, once peruses:hop.Propagation after the change
track_benchmarksjob fails on alerts and is rendered red on the run, so the signal stays visible.--error-on-alertis untouched, as is the--github-actionscheck that reports the alert.continue-on-error: truekeeps that failure from failing the workflow, soTrack Benchmarksconcludessuccess.track_benchmarkscaller job intest.ymltherefore succeeds. Nothing intest.ymlhasneeds: [track_benchmarks], so no downstream job is skipped, andTestconcludessuccess.needs.test.result == 'success'inci.yml, sodeployruns andci-successpasses.ci-successenumerates job results one by one, but only at the top level (lint,cli,runner,publish_runner_canary,test,build,docker,deploy,release). There is no per-benchmark entry to adjust, so the single line is enough.track_pr_benchmarks.ymlis left aloneThe PR side cascade gates nothing.
Track PR Benchmarksis a separateworkflow_runworkflow, no job needs it, it is not amongci-success's dependencies, and PR runs do not deploy. Its red X on an alert is the intended pull request signal, so it keeps--error-on-alertand stays gating-free by construction.