Repository navigation
Conversation
THE PERFORMANCE TESTS HAVE NEVER RUN ON MASTER The `test` job carrying the required "Performance Tests" check is pull_request-only, and master's push build runs bin/ci-build.sh with -Dexcluded.groups=performance. So every throughput figure this project has ever had came from a PR run, against no trunk series at all. That one gap is why three separate things were stuck: no baseline to choose a regression threshold from, no way to separate a PR's slow number from a slow runner, and no master-side run to put opposite a branch - so a like-for-like comparison had to be MANUFACTURED with a throwaway PR from a branch at master's tip rather than found in history. A new perf-baseline workflow runs the lane on master pushes, and on demand. It is deliberately not a leg of the `build` job: that job gates SNAPSHOT publishing, and a performance result must never hold a release or block one, least of all a result that is explicitly not a gate. AND THE NUMBERS NOW SURVIVE THE RUNNER Until now bin/performance-test.sh wrote its summary inside the runner and printed it, and nothing uploaded it - the only surviving copy was the PC-THROUGHPUT lines in the job log, which is exactly the artefact docs/solutions/workflow-issues/gh-run-view-log-truncation.md exists about. Both lanes now upload it, at 90 days rather than the chaos lane's 14: those reports diagnose one failure, these ARE the accumulation, so a short window defeats the point. WHAT THIS IS NOT A gate, and not the end of the throughput-regression-gate question either. That work defers a threshold until somebody has seen the real spread; an artifact is per-run and still expires, so building a distribution still means downloading several runs by hand. The precondition is improved, not met, and the workflow header says so rather than leaving the next reader to discover it. Storing a durable series was considered and declined - no data branch, no contents: write grant on a public repo, no third-party benchmark action (operator decision, 2026-09-01). Scope was decided the other way: the whole lane runs on master, not just the throughput classes, so a regression in any performance test is visible rather than only in the one somebody happened to be investigating. No concurrency group, matching chaos-pain.yml and experiments.yml: a group keeps one running plus one pending and discards the rest, which would silently drop exactly the measurements this exists to take.
…and report rates from all four classes WHAT THIS ADDS TO THE COLLECTION ALREADY ON THIS BRANCH bin/check-throughput-regression.sh compares MultiInstanceHighVolumeTest's rate against docs/perf-baseline.tsv AFTER cancelling machine speed using the other performance classes in the same run. Raw comparison against a stored number is mostly a measurement of which runner turned up - the same code has been seen at 109,898, 82,505 and 71,387 records/second - so a bound tight enough to catch a regression would fire constantly. Comparing against neighbours removes that, and it is what actually settled the #29 question by hand: neighbours +5% and +4% while the subject was -39%. THE NEIGHBOURS NOW REPORT RATES, NOT JUST DURATIONS ThroughputReport.report had exactly ONE call site. LoadTest, VeryLargeMessageVolumeTest and LargeVolumeInMemoryTests ran in this lane and emitted nothing - which bin/performance-test.sh already called out as NOT MEASURED. All three now report, on the failing exit as well as the passing one, since a run that missed its deadline is the one whose rate a collector most wants and the one that would otherwise be absent from every series. Each measures from the START OF THE WAIT rather than method entry: producing the fixture and standing up a broker is machine time, not product time. The check still normalises on failsafe class TIMES, which every class has, and which - unlike rates - exist in every CI log this project has ever produced. Prefer rates as they accumulate: a rate measures the product, a class time measures the run and includes container pulls and topic creation. WHAT IT CATCHES, AND WHAT IT CANNOT It catches a regression that hits the throughput test harder than its neighbours. It is blind to one that slows everything equally, because within-run data cannot distinguish that from a slow runner. Stated in the script header rather than discovered later. THE THRESHOLDS ARE COARSE AND THAT IS NOT YET GOOD ENOUGH Worked through on the #29 numbers the ratio lands near 0.64, which WARNS and does not fail. The 2x fail bound catches the four-to-tenfold class that sat unnoticed in this lane for weeks, and nothing tighter can be justified until somebody has measured the spread of the NORMALISED ratio. docs/inflight/perf-validate-the-regression-check-against-a-known-defect.md records the red/black experiment that would settle it, with its prediction written down BEFORE the run - a detector evaluated after seeing its output is evaluated against whatever it happened to say. MACHINE FINGERPRINT AS METADATA, NEVER AS AN INPUT bin/performance-test.sh now writes a cpu/cores/memory comment line ahead of the data. Nothing branches on it. It exists so somebody can CHECK the normalisation is working: if the normalised ratio turns out to correlate with CPU model, the normalisation is failing, and this is how that gets noticed. *.tsv joins ENFORCED_TYPES in the copyright checker rather than being exempted: a new file type arrived, and the checker is built to fail on an unclassified one rather than skip it quietly.
…29 nearly merged now FAILS THE SPREAD WAS ALREADY RECORDED - IN LOGS NOBODY HAD MINED A throughput gate has been deferred since August waiting for "a few runs of real spread", and waiting could never supply it because nothing accumulated the spread. It did not have to be built forward: every performance run inside GitHub's log-retention window has its per-class failsafe times, and recent ones have rates, sitting in the job log. bin/perf-backfill.sh recovers them - read-only, via the run-logs ARCHIVE and never `gh run view --log`, which silently truncates and produced a confident wrong conclusion during the investigation that led here. Output goes to a local history file outside the tree, so the decision not to keep a series in the repository stands. WHAT THE HISTORY SAYS Twelve runs carried a rate, and the normalised ratio separates with a gap and nothing inside it: 0.407 0.475 0.476 0.507 0.578 0.580 0.591 0.605 every regressed run 0.778 0.922 1.000 1.000 every healthy run FAIL_BELOW is 0.70, near the middle of that gap. The shortfall #29 was about to merge scores 0.578, so it FAILS the lane rather than warning - which the previous commit could not justify and which is the whole point. WARN_BELOW at 0.85 fires on the slowest healthy run seen (0.778), deliberately: a warning means look at this, not this is broken. THE FIRST RATIO WAS WRONG AND THE DATA SAID SO bin/perf-backfill.sh's first version divided by the sum of every OTHER class that ran. That denominator swung from 130s to 1335s across branches - not because a machine was ten times slower, but because different branches run different performance classes. It conflated "the subject slowed" with "the lane changed", which is the one thing this measure exists to keep apart. A fixed neighbour set fixes it, and the defect was only visible because the backfill was actually run rather than reasoned about. THE SELF-TEST IS THE EVIDENCE bin/test-check-throughput-regression.sh pins six cases, four of them REAL observations from named CI runs: the 43,552 regression, the 29,372 worst case, the same branch at 76,950 after the one-line fix, and the slowest healthy run at 57,215 which must warn and must not fail. A threshold whose justification lives only in a commit message is one nobody can re-derive; these fail if it drifts. The other two cases are the not-a-pass paths - a summary with no rate, and no neighbour to normalise against - because a lane that measured nothing must never be reported as a lane that found nothing. CORROBORATION FROM THE OTHER DIRECTION #29 independently ran the control arm and reached the same mechanism: one main-code term changed, neighbours within 1-3%, and the lane-composition confound moved the wrong way and it still improved. WHAT THIS DOES NOT ESTABLISH, AND IT MATTERS Eight of the twelve observations are the same branch and the same defect, so the regressed group is one phenomenon sampled eight times, not eight independent regressions. And a regression that slows everything equally stays invisible: within-run normalisation cannot separate that from a slow runner, by construction.
…e tightened yet "A UNIFORM SLOWDOWN IS INVISIBLE" WAS WRONG AS WRITTEN It is invisible to THIS CHECK, which normalises within a run. It is not invisible to the available data: every performance run inside GitHub's log-retention window is queryable, and bin/perf-backfill.sh already recovers it. The limit is that nothing reads across runs yet, which is a gap rather than a structural impossibility - and the difference matters, because the first framing closes an avenue the second one opens. "INDEPENDENTLY" WAS WRONG TOO #29 did not rediscover the mechanism separately; it merged the branch carrying the fix (5ed8856) and then measured it. That is one team measuring one change, and calling it an independent arrival inflated the evidence. The controlled measurement it did run is worth more than a rediscovery would have been, so the correction costs nothing but accuracy. 0.70 IS MEASURED, NOT TIMID - AND THE MEASUREMENT WAS A SURPRISE The obvious reading of the healthy band, 0.778 to 1.000, is that most of that spread is a stale baseline, curable by comparing against master runs near a PR's merge-base. It is not. The 0.778 run is docs/beads-evaluation, which is DOCS-ONLY - four markdown files, zero main-code change - so a tree byte-identical to master in every line of code lost 22% AFTER normalisation. So anything tighter than about 0.72 fails documentation PRs, and a gate that does that is switched off within a week. It also says the proportionality assumption underneath neighbour-normalising is only approximately true: in that run the neighbours were 6% slow while the subject was 27% slow, so the subject amplifies machine variance rather than tracking it - plausibly because it is the twelve-instance KEY-ordered case, though that is a hypothesis and not a finding. Tightening therefore needs comparison against a DISTRIBUTION of master runs rather than any single point, since averaging is the only thing that removes noise of that shape. THE DESIGN THAT GETS THERE IS RECORDED RATHER THAN BUILT docs/inflight/perf-a-queryable-history-instead-of-a-single-committed-baseline.md carries the operator's proposal: each run downloads the previous aggregate artifact, adds only what it lacks, and re-uploads the whole aggregate, so the series lives in the newest run rather than in any expiring one. No data branch, no write grant, and retention only bites if nothing runs for a whole window. It also fixes the baseline never rising when the product gets FASTER, which a hand-updated file cannot.
…, to watch the gate fail DO NOT MERGE. This branch exists to be thrown away. bin/check-throughput-regression.sh's thresholds were derived from historical runs and its self-test pins those observations, but nothing has yet watched the check go red in CI on a tree that is actually regressed. Everything to date is the check applied to recovered numbers, which is one step short. So the defect is reintroduced verbatim: the O(shards) accessor passed as a plain log argument in the control loop, evaluated on every pass at every log level, exactly as #29 carried it. One term differs from the branch this was cut from, and nothing else. PREDICTION, WRITTEN BEFORE THE RUN: the ratio lands in 0.407-0.605, the band every regressed run measured so far occupies, and the check exits 1. If it lands above 0.70 the check does not work and that is the more valuable result of the two.
Dependency ReviewThe following issues were found:
License Issues.github/workflows/perf-baseline.yml
OpenSSF Scorecard
Scanned Files
|
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its |
|
astubbs
added a commit
that referenced
this pull request
Sep 1, 2026
… weaker claim than expected THE DETECTOR WORKS END TO END, WHICH WAS THE OPEN ITEM Red arm (#402, the defect reintroduced verbatim) scored ratio 0.336 and exited 1. Black arm (the same tree without it) scored 0.861 and passed. Both on real hosted runners, running the real lane. Until now every claim about this check was the check applied to numbers recovered from logs; it has now been watched going red on a genuinely regressed tree. THE PREDICTION WAS DIRECTIONALLY RIGHT AND NUMERICALLY WRONG It said 0.407-0.605, the band every historical regression occupied. Actual was 0.336, below that band, because the injected defect ran on a tree where no other lane change masked it. Recorded rather than rounded to "as predicted" - which is the entire reason for writing a prediction down first. AND THE FINDING THAT MATTERS MORE: NOTHING HERE PROVES ADDED DETECTION POWER The red arm struck the 60-second GATING_CEILING as well, so the pre-existing wall-clock assertion failed that run too. Checking back through the history, EVERY regression observed so far also breached the ceiling. So on the evidence to date this check has never caught anything the deadline would have missed, and the note now says so where the result is, rather than leaving that inference to a reader. What it demonstrably adds is narrower and worth claiming as exactly that: a graded number instead of a binary, and the ability to tell a healthy tree on a slow runner from a real regression - which is the confusion that cost this project weeks. The band under the ceiling, where a real regression still finishes inside 60s, is design intent and not yet a demonstrated catch. The experiment that would demonstrate it is an injected slowdown sized NOT to breach the ceiling, and that is recorded as the better next run. MORE EVIDENCE THAT 0.70 CANNOT TIGHTEN The black arm scored 0.861 on a tree differing from the baseline run only by docs and scripts. With the docs-only run at 0.778 and an earlier run of the same branch at 1.000, that is 0.778 to 1.000 on effectively identical code across three runs. The floor is noise, not sensitivity.
Owner
Author
|
Result recorded: red arm ratio 0.336, The detector is confirmed to fire end-to-end in CI on a genuinely regressed tree, which was the open item. Two honest caveats, both recorded in
Closing unmerged, as intended. |
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.
Description
DO NOT MERGE. Throwaway branch, opened to be watched and then closed.
bin/check-throughput-regression.shon #401 has thresholds derived from historical CI runs, andbin/test-check-throughput-regression.shpins those observations as cases. But nothing has yet watched the check go red in CI on a tree that is actually regressed — everything so far is the check applied to recovered numbers, which is one step short of the thing itself.This branch is #401 plus one term: the defect #29 carried, reintroduced verbatim — an O(shards) accessor (
wm.getNumberOfWorkQueuedInShardsAwaitingSelection()) passed as a plainlog.traceargument in the control loop, so it is evaluated on every pass at every log level.Prediction, written before the run
The normalised ratio lands in 0.407–0.605, the band every regressed run measured so far occupies, and
Throughput regression checkexits 1.If it lands above 0.70 the check does not work, and that is the more valuable of the two results. A detector evaluated after seeing its output is evaluated against whatever it happened to say, so the prediction is recorded here first.
Why it has to be a PR
The performance lane is
if: github.event_name == 'pull_request', so a branch push alone runs nothing. Draft, and closed as soon as the run reports.issue-refs: N/A - throwaway validation branch, closed unmerged
Checklist
N/A - throwaway branch, never mergesdocs/features/-N/A - throwaway branchN/A - this branch IS the test; it is a live-fire check of astubbs/parallel-consumer#401's gatece-simplifyandce-code-reviewlocally -N/A - one deliberately-defective line, never merging