feat(bench): peak throughput under an SLO, and fairness scoped to comparison groups - #53
Merged
Conversation
…er family
Every healthy target reported ~485 rps because the paced mixed workload caps
offered load at `VUs / think_time`. That number measures the load generator's
sleep timer, not the database: a tenfold service-time difference moves it ~8%.
Capacity is a different question and now has its own suite.
Saturation suite
----------------
`workload.saturation.v1.json` (and a preview-sized variant) declare an unpaced
stepped concurrency ramp — 4 to 1024 VUs, doubling, 20 s holds — plus the SLO the
headline is quoted with. The steps are the hold stages themselves, so the ramp
has exactly one definition.
The load generator now emits one aggregate per hold plateau to `$BENCH_STEPS_OUT`,
computed from that plateau's merged raw samples, so a step's p99 is a real p99
rather than a median of per-second p99s. A saturation run without that artifact
fails instead of approximating it.
The headline is the highest-concurrency step that met the SLO and stayed within
`limits.err`, and exactly one of three named outcomes is always recorded:
saturated a peak was found and a later step failed to qualify
did_not_saturate the last step still qualified — the top step is a lower
bound and the ramp needs extending, not a peak
slo_never_met no step qualified, so there is no peak and none is reported
Steps over the error limit are disqualified from being the peak, stay in the
curve, and carry the reason. The whole curve — concurrency, rps, p50/p90/p95/p99,
error rate, CPU — is published, because it is what makes the headline checkable.
The previous `saturation: {knee_rps, knee_p95}` is removed rather than kept
beside the new block. It ran on paced workloads, where the sleep-timer ceiling
means its "knee" described the load generator; and it fell back to the
highest-throughput bucket when detection failed, reporting "no knee found" as a
knee. `did_not_saturate` is the honest form of that case. This is a breaking
artifact change: runs recorded before it no longer validate, and consumers read a
missing `saturation` as "not measured".
Two-axis fairness
-----------------
Targets now declare `fair.family` and `fair.tuning`. Within a family the harness
— workers, pool, tuning — must be identical or the run fails with exit 3 before
any load is generated, naming both targets and the differing field. The previous
behaviour was a warning in the manifest, which is silent in every way that
matters: it still let an unequal library comparison ship.
Across families nothing is constrained, because those differences are the stack
comparison. `manifest.harness` records the verified configuration per family so a
reader cannot mistake a stack difference for a library one. In-process-cache
targets stay exempt from the equality check and are listed rather than dropped.
Verified end to end against the sqlite family: all four targets reach
`saturated`, and the peaks separate them (26.6k / 25.5k / 24.2k / 18.3k rps),
so the ramp measures the library rather than the harness.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Peak throughput now means the most throughput. The previous rule took the highest-concurrency step that held the SLO, which on a non-monotone closed-loop curve is often slower than an earlier step that also held it: a measured drizzle-rs SQLite ramp did 31457 rps at 16 VUs and 28760 at 256, both inside a 25 ms p99. Reporting the latter as "peak throughput at p99 < 25 ms" understated the target by 9% and pointed at a worse operating point on both axes. Ties break toward the lower concurrency — the same throughput for fewer in-flight requests is strictly better. `lower_bound_rps` is likewise the best qualifying throughput rather than the top step's. `outcome` is unchanged and deliberately independent of where the maximum landed: it depends only on whether the ramp's last step still qualified. A ramp can peak early, flatten, and still be `did_not_saturate` — a visible bend is not a measured limit. Unit tests pin the non-monotone case that distinguishes the two readings, the tie-break, and an early maximum that must not be read as saturation. fair.family is a comparison group, not an engine ------------------------------------------------ A family is the set of targets claiming to be directly comparable. It usually maps onto the database engine but splits where the harness cannot honestly be equalised, and merges where it can: - `sqlite-ts` (bun:sqlite) splits from `sqlite`. bun:sqlite is synchronous on a single-threaded runtime, so a pool of 8 there is theatre; forcing one would cripple the target in the name of fairness. drizzle-rs-on-rusqlite versus drizzle-orm-on-Bun differs in language, runtime and concurrency model, which makes it a stack comparison. Inside the group, drizzle-orm versus bun:sqlite is a real library comparison. - `postgres-ts` splits from `postgres` for the same reason. - `postgres` deliberately spans two spec files: the builtin drizzle-rs and tokio-postgres targets and the sqlx/diesel/sea-orm/toasty file are all Rust on an identical harness, and that is the most valuable library comparison in the suite. Grouping is declared per target precisely so it is not inferred from file layout. Enforcement and delta scoping follow `fair.family`; presentation does not. Both SQLite groups still belong in one table with SQLite in the database column. The vocabulary is one key space shared by `fair.family` in target.v1 and both `harness[].family` and `target_meta[].fair.family` in run-manifest.v1, defined once per schema and referenced. Consumers join harness lookup on it, so drift would silently render every affected row as "harness not declared" — a false negative on exactly the disclosure the block exists to provide. A test fails if the two schema definitions diverge, if a checked-in target declares a family outside the vocabulary, or if a run emits a harness key that matches no target. Artifacts carry ids only; display labels stay with the presentation layer, because a name frozen into a published artifact can never be reworded. Re-verified end to end against the sqlite family. All four targets still reach `saturated`, and the corrected peaks land where throughput is actually maximal: rusqlite-prepared 28853 rps @16 (p99 1.41 ms), drizzle-rs-query 28792 @16 (1.22 ms), drizzle-rs 25098 @64 (4.14 ms), rusqlite-unprepared 18094 @256 (16.95 ms). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
…t per run Within-family identity was enforced per run, but a family can span runs: `postgres` covers both targets.postgres.v1.json and targets.postgres-rust-orms.v1.json, and outside the publish topology those execute as separate CI jobs producing separate artifacts. Each run would check only its own shard and pass, so a pool changed in one file would not surface until a consumer merged the two and marked the family unverified — after publish, in someone else's UI, which is the same "silent unequal comparison" the hard error exists to prevent. The same enforcement now also runs over the union of every checked-in targets.*.json as a unit test, so the drift fails CI at source with the message that names both targets and the field. Verified by injecting a pool change into one postgres-rust-orms target and confirming the test fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
The paced suite caps offered load at
VUs / think_time, so every healthy target reported ~485 rps — the throughput column measured the load generator's sleep timer, not the database. A 10x service-time difference moved it ~8%.Saturation suite. Unpaced stepped concurrency ramp (4→1024 VUs, geometric), 20s holds, SLO declared in the spec (
p99 < 25ms). The headline is the fastest qualifying step — max throughput among steps that held the SLO and stayed inside the error budget, ties toward lower concurrency. Step sizing was derived from an exploratory ramp, not intuition: the knee moves >10x between embedded SQLite and PostgreSQL, so geometric spacing brackets it within 2x across a 256x range, and the ramp must reach 1024 so the fastest stack still breaches — otherwisedid_not_saturatemeans nothing.Result on the sqlite family: targets now spread 18.1k–28.9k rps (unprepared rusqlite 37% below the leaders) where the paced suite put all four at ~485. It measures the library.
No fallbacks. Three first-class terminal outcomes —
saturated,did_not_saturate("at least N req/s, knee not reached"),slo_never_met(no number substituted). Steps disqualified by error rate carry their reason. The legacyknee_rps/knee_p95heuristic is deleted: it ran on paced runs where the ceiling made its knee an artifact of the sleep timer, and it silently fell back to the max-rps bucket when detection failed —did_not_saturateis the honest form of that case. Missing percentiles are a hard error rather than being filled from a neighbour.Fairness, two meanings. Within a comparison group the harness must be identical — hard failure naming both targets and the differing field, raised before any load is generated. Across groups configs may differ freely; that difference is the stack comparison, and each group's harness is recorded in the manifest for display.
fair.familyis a declared comparison group, not an inferred engine: two spec files can share thepostgresgroup (all nine Rust PG targets do), and one engine can hold two groups where harnesses genuinely cannot be equalised (sqlitevssqlite-ts— bun:sqlite is synchronous on a single-threaded runtime, so a pool of 8 there would be theatre). One closed enum shared by all three schema sites, enforced by tests.83 runner tests (63 unit + 20 integration), fmt/clippy clean, real end-to-end saturation run revalidated against the new schemas, and the dashboard branch's conformance fixtures pass this validator.
🤖 Generated with Claude Code