Skip to content

Make the metric single valued - #989

Merged
epompeii merged 8 commits into
u/ep/benchmark-parameters/parameter-tablefrom
u/ep/benchmark-parameters/single-valued-metric
Aug 23, 2026
Merged

Make the metric single valued#989
epompeii merged 8 commits into
u/ep/benchmark-parameters/parameter-tablefrom
u/ep/benchmark-parameters/single-valued-metric

Conversation

@epompeii

@epompeii epompeii commented Aug 22, 2026

Copy link
Copy Markdown
Member

Layer 2 of the benchmark parameters stack. Stacked on #988; targets u/ep/benchmark-parameters/parameter-table and retargets devel when that merges.

A metric row is now one named scalar. metric becomes (report_benchmark_id, measure_id, name, value), with name sitting among the identity columns and the lower_value and upper_value columns dropped. Uniqueness extends to (report_benchmark_id, measure_id, name). The metric triple is not a shape the table knows about any more: it is a convention over three names, value, lower_value, and upper_value, each an ordinary row. Nothing about this reaches the wire, and nothing about it is meant to be visible.

Nothing changes for anyone

Every response that reads a metric is byte identical across the migration, and the tests prove it with captured bytes rather than an assertion. metric_migration.rs seeds a database in the shape the migration finds, captures the raw bodies of the perf query, the metric endpoint, the report, and the alerts, runs the migration, and captures them again. The two captures must be equal byte for byte, not equal after parsing: parsing and re-serializing would hide exactly the float formatting drift this layer exists to rule out.

The fixtures are chosen to break a careless implementation. Metrics with both bounds, the lower bound only, the upper bound only, and neither, spread over two measures and two report benchmarks, so a pivot that joins on the wrong key leaks a bound across a row and fails. The floats are the ones that expose formatting drift, not round numbers. The rows are seeded with explicit, non-contiguous ids, because real metric ids have gaps in them: deleting a report cascades its metrics away. A fixture numbered from one cannot tell a migration that keeps every id from one that reassigns them, since INSERT ... SELECT into an empty table hands back the same numbering either way.

One binary serves both captures because every reader goes through the metric_boundary view, whose column list the migration holds unchanged. That is the whole trick of this layer: the view pivots the named rows back into the three columns its readers know, so the perf query, the metric endpoint, the report, and the alerts are untouched. The two alert queries that read the metric table directly move onto the view, which is what makes them read either shape.

Detection samples the point estimates

A threshold's sample is read straight off the metric table, filtered by measure. A bound is a named row under that same measure now, so the filter has to name the point estimate too. Without it the sample becomes a mixture of measurements and the limits previously computed from them, the mean and standard deviation are taken over that mixture, and a bounded max_sample_size fills with a fraction of the history it asked for. Detection has only ever gated the value scalar, and the sample it is gated against is the point estimates alone.

detection_samples_only_the_point_estimates pins it end to end: two projects with the same point-estimate history, one of which also carries bounds, must produce the same boundary for the same next report. Before the filter, the same fixture computed a baseline of 12.0 in one and 1004.0 in the other.

The migration

Each old row explodes into up to three. The value row keeps the original id and uuid, which makes the boundary remap a no-op: boundary.metric_id already points at that id, so no boundary row is rewritten and its unique constraint is undisturbed. Each stored bound becomes a new row with a minted uuid.

The minted uuids are v7 shaped. The 48 bit millisecond prefix is computed once into a temporary table and cross joined into both bound inserts, so every uuid the migration mints is a v7 of the one instant the migration ran at. The rest of the uuid is randomblob, with the version nibble set to 7 and the variant nibble drawn from 89ab by random() & 3 rather than abs(random()) % 4, because abs(-9223372036854775808) is an integer overflow error in SQLite. The prefix comes from julianday rather than unixepoch('now', 'subsec'), which is newer than the oldest SQLite that can otherwise run this migration. An earlier revision of this description argued the shared prefix for index locality, and that argument is retracted: the uuid index is now built after the rows land, by an external sort that is indifferent to where its keys fall, so neither the shared prefix nor the v4 uuids the preserved rows carry over cost the build anything. The timings that argument cited were measured against the online build and no longer describe this migration. The prefix stays because it is what makes a minted uuid a v7, not because of what it costs.

down.sql is a true inverse: the bound rows fold back into the columns they came from. Names outside the triple have no column to land in and are dropped, which is the price of going back to a shape that cannot hold them.

One known gap

A lower_value or upper_value row's own uuid does not resolve through the pivoted view, so GET /v0/projects/{project}/metrics/{uuid} on one of them is a 404. This is acceptable and is not fixed here. No uuid that existed before the migration is affected, because the value row keeps its own, and the uuids the migration mints have never been exposed anywhere: nothing reads them, nothing returns them, and no client has ever seen one. When a payload can name its own scalars, the endpoint gets a shape that can address them.

The unique keys are built after the rows land

up_metric is created without its two unique constraints, and they arrive after the third insert as named unique indexes on the same table, which the rename then carries over:

CREATE UNIQUE INDEX index_metric_uuid ON up_metric(uuid);
CREATE UNIQUE INDEX index_metric_report_benchmark_measure_name ON up_metric(report_benchmark_id, measure_id, name);

Declared on the table, the two indexes are maintained online across every insert the explosion produces, two unique B-trees held dirty a page split at a time, and the value rows carry their original uuids, which are v4, so each one lands on a random page of the uuid index. Built after the rows land, each is one external sort. Measured over the whole rebuild on a synthetic database at production's page size and cache ratio, deferring them takes page reads from 179,939 to 11,222 and page writes from 181,037 to 6,146: 16 times fewer reads and 29 times fewer writes.

What is enforced does not move. A named unique index and a table constraint are the same index to SQLite, down to the UNIQUE constraint failed: metric.uuid it reports on a collision, so the pivot rides the same key and the query plan above is the same plan. down.sql needs no change: it recreates the pre-migration table with its own constraints, and its DROP TABLE metric takes the named indexes with it. The round trip test asserts the two index names it finds after a down and up.

Query plan

The view's self joins ride index_metric_report_benchmark_measure_name. Because that index makes them at most one row, SQLite omits them outright for any query that does not select a bound. Measured with EXPLAIN QUERY PLAN on an ANALYZEd database before and after the migration, the pivot adds exactly two fully bound index seeks and no scan:

-- selecting the bounds
SEARCH metric USING INDEX index_metric_report_benchmark_measure_name (report_benchmark_id=? AND measure_id=? AND name=?)
SEARCH lower_metric USING INDEX index_metric_report_benchmark_measure_name (report_benchmark_id=? AND measure_id=? AND name=?) LEFT-JOIN
SEARCH upper_metric USING INDEX index_metric_report_benchmark_measure_name (report_benchmark_id=? AND measure_id=? AND name=?) LEFT-JOIN
SEARCH boundary USING INDEX sqlite_autoindex_boundary_2 (metric_id=?) LEFT-JOIN

-- not selecting the bounds
SEARCH metric USING INDEX index_metric_report_benchmark_measure_name (report_benchmark_id=? AND measure_id=? AND name=?)

Everything else in the plan is unchanged, and the driving metric step tightens from two bound columns to three.

Synthetic fixture run

A throwaway database built by applying every earlier migration to an empty file and seeding 3 branches, 2 testbeds, 100 benchmarks, 3 measures, 300 versions, 600 reports, 120,000 report benchmarks, 360,000 metrics, 72,000 boundaries and 18,000 alerts. Bounds are spread across all four cases: half the metrics carry both, a tenth the lower only, a tenth the upper only, and the rest neither.

Rows before 360,000
Rows after 792,000 (360,000 value, 216,000 lower_value, 216,000 upper_value)
Ratio 2.20
Apply 2.79s in one transaction
Revert 0.71s
Re-apply 2.85s
PRAGMA foreign_key_check empty, 0.12s
PRAGMA integrity_check ok, 1.33s
File before / write-ahead log at commit / file after 73 MiB / 109 MiB / 181 MiB

Every check held: the boundary table dump is byte identical before and after, the full 14 column metric_boundary dump is byte identical before and after and again after a down and up round trip, count(DISTINCT uuid) equals count(*), the sqlite_master diff shows only the metric table and the view, and all 432,000 minted uuids carry version nibble 7, a variant nibble uniformly drawn from 89ab, and exactly one shared millisecond prefix.

The write-ahead log growing larger than the database it started from is the thing to plan around: this is one write transaction that holds the old table, the new table and the log at once.

Billing does not move

QueryMetric::usage counts the point estimate only. Named values collapse into their measure's series, so a bounded metric bills as one measurement, exactly as it did when it was one row. In this shape every measure has exactly one value row, which makes the count identical by construction. The billing layer revisits this when a payload can name p99 and never name value.

usage_does_not_count_the_bound_rows holds that: it seeds a metric with both bounds and asserts three rows still bill as one. Removing the filter fails it.

Draft until the dry run

This rebuilds the whole metric table, so it stays a draft until a timed migration dry run against production scale data comes back clean. It is not to be marked ready before then. Results go here when it runs.

The dry run works on a copy, never in place. Foreign keys must be off on the connection and outside any transaction before anything else, because PRAGMA foreign_keys is a no-op inside a transaction and boundary.metric_id REFERENCES metric (id) ON DELETE CASCADE: if they are enforced when DROP TABLE metric runs, every boundary and every alert hanging off it goes with it. The API server already does this around startup migrations.

cp snapshot.db dryrun.db
MIG=lib/bencher_schema/migrations/2026-08-16-120000_single_valued_metric
{ echo "PRAGMA foreign_keys=off;"; echo "BEGIN;"; cat $MIG/up.sql;   echo "COMMIT;"; } > apply.sql
{ echo "PRAGMA foreign_keys=off;"; echo "BEGIN;"; cat $MIG/down.sql; echo "COMMIT;"; } > revert.sql

VIEW="SELECT metric_id,metric_uuid,report_benchmark_id,measure_id,value,
  coalesce(lower_value,'~'),coalesce(upper_value,'~'),coalesce(boundary_id,'~'),
  coalesce(boundary_uuid,'~'),coalesce(threshold_id,'~'),coalesce(model_id,'~'),
  coalesce(baseline,'~'),coalesce(lower_limit,'~'),coalesce(upper_limit,'~')
  FROM metric_boundary ORDER BY metric_id;"
BOUNDARY="SELECT id,uuid,metric_id,threshold_id,model_id,baseline,lower_limit,upper_limit
  FROM boundary ORDER BY id;"
MASTER="SELECT type,name,tbl_name,coalesce(sql,'') FROM sqlite_master ORDER BY type,name;"

sqlite3 dryrun.db "SELECT 'metric',count(*) FROM metric
  UNION ALL SELECT 'with_lower',count(*) FROM metric WHERE lower_value IS NOT NULL
  UNION ALL SELECT 'with_upper',count(*) FROM metric WHERE upper_value IS NOT NULL
  UNION ALL SELECT 'boundary',count(*) FROM boundary
  UNION ALL SELECT 'alert',count(*) FROM alert;"
sqlite3 dryrun.db "$VIEW" > view_pre.txt
sqlite3 dryrun.db "$BOUNDARY" > boundary_pre.txt
sqlite3 dryrun.db "$MASTER" > master_pre.txt

time sqlite3 -bail dryrun.db < apply.sql

sqlite3 dryrun.db "SELECT name,count(*) FROM metric GROUP BY name;
  SELECT 'total',count(*) FROM metric;
  SELECT 'distinct_uuid',count(DISTINCT uuid) FROM metric;"
sqlite3 dryrun.db "$VIEW" > view_post.txt;         cmp view_pre.txt view_post.txt
sqlite3 dryrun.db "$BOUNDARY" > boundary_post.txt; cmp boundary_pre.txt boundary_post.txt
sqlite3 dryrun.db "$MASTER" > master_post.txt;     diff master_pre.txt master_post.txt
sqlite3 dryrun.db "PRAGMA foreign_key_check;"
sqlite3 dryrun.db "PRAGMA integrity_check;"

time sqlite3 -bail dryrun.db < revert.sql
sqlite3 dryrun.db "$VIEW" > view_reverted.txt;  cmp view_pre.txt view_reverted.txt
time sqlite3 -bail dryrun.db < apply.sql
sqlite3 dryrun.db "$VIEW" > view_roundtrip.txt; cmp view_pre.txt view_roundtrip.txt
sqlite3 dryrun.db "PRAGMA foreign_key_check;"; sqlite3 dryrun.db "PRAGMA integrity_check;"

Expected: value rows equal the metric count before, lower_value rows equal the with_lower count, upper_value rows equal the with_upper count, so the ratio is 1 + (with_lower + with_upper) / metric and is bounded above by 3. Every cmp is silent. The sqlite_master diff shows the metric CREATE TABLE, the metric_boundary CREATE VIEW, and the index rows: metric carries index_metric_uuid and index_metric_report_benchmark_measure_name and no autoindex at all, where before it carried sqlite_autoindex_metric_1 and _2 and no named index. foreign_key_check returns nothing, integrity_check returns ok, and count(DISTINCT uuid) equals count(*).

A failure is any of: a non-silent cmp, a sqlite_master difference beyond those two objects, any row from foreign_key_check, anything but ok from integrity_check, a value row count that does not equal the metric count before, count(DISTINCT uuid) below count(*), an error from either script, or a wall clock beyond the maintenance window budgeted for it.

Wall clock
Rows before / after
Bound rows created
Boundary rows rewritten expected 0
PRAGMA foreign_key_check expected empty
PRAGMA integrity_check expected ok
Response equivalence spot check

Tests

Tests are committed first and fail there. 536d408 carried the first round: the equivalence test failed on the perf response, the explosion test on its row count, and the ingest test on the bound that never came back. perf_lower_upper_values went red alongside them and green with the implementation.

afe70b7 carries the second round. detection_samples_only_the_point_estimates failed on the boundary, 12.0 against 1004.0 at the baseline, and migration_explodes_the_metric_triple_into_named_rows failed on the uuid version nibble, 4 against 7. The rest of that commit hardens tests that already passed against mutations they should have caught and did not: the fixture's non-contiguous ids, following the boundary through to the measurement it was created against rather than observing that it was not rewritten, anchoring the round trip against the pre-migration capture so a symmetric error cannot cancel itself out, asserting the indexes and the view's column list and both integrity pragmas after the round trip, and asserting that a duplicate name under one measurement collides.

The mutation those were written for is deleting id from the value row insert so the migration renumbers what it carries over. Before, nothing caught it. Now three tests do: the explosion test, the byte equivalence test, and the round trip.

The parameter migration is no longer the last one, so its tests revert down to it by version rather than reverting whatever happens to be on top.

CI

The gate counts below predate the commit that defers the index build.

The workflow only runs for pull requests based on main, cloud, or devel, so a stacked pull request gets no run until it retargets devel. Every gate was run locally in the meantime: cargo fmt --check, cargo clippy --no-deps --all-targets --all-features -- -Dwarnings, cargo check --no-default-features, cargo nextest run --workspace --all-features (2017 passed, 3 skipped), cargo nextest run -p bencher_schema --features plus (190 passed), cargo test --doc --workspace --all-features, cargo deny check, the bencher_valid WASM build, and cargo gen-types, which produces no diff because nothing here reaches the wire.

@epompeii
epompeii force-pushed the u/ep/benchmark-parameters/single-valued-metric branch from 3bfc346 to f2a830d Compare August 22, 2026 16:27
@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

馃惏 Bencher Report

ProjectBencher
Branchu/ep/benchmark-parameters/single-valued-metric
Testbedintel-v1
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
microseconds (碌s)
(Result 螖%)
Upper Boundary
microseconds (碌s)
(Limit %)
Adapter::Json馃搱 view plot
馃毞 view threshold
4.75 碌s
(+1.28%)Baseline: 4.69 碌s
4.99 碌s
(95.09%)
Adapter::Magic (JSON)馃搱 view plot
馃毞 view threshold
4.63 碌s
(+1.77%)Baseline: 4.55 碌s
4.81 碌s
(96.30%)
Adapter::Magic (Rust)馃搱 view plot
馃毞 view threshold
25.77 碌s
(-0.28%)Baseline: 25.84 碌s
27.10 碌s
(95.07%)
Adapter::Rust馃搱 view plot
馃毞 view threshold
3.50 碌s
(-0.64%)Baseline: 3.53 碌s
3.94 碌s
(89.04%)
Adapter::RustBench馃搱 view plot
馃毞 view threshold
3.50 碌s
(-0.83%)Baseline: 3.53 碌s
3.94 碌s
(88.78%)
馃惏 View full continuous benchmarking report in Bencher

@epompeii
epompeii force-pushed the u/ep/benchmark-parameters/single-valued-metric branch 2 times, most recently from d54fa02 to 244a3b3 Compare August 23, 2026 03:21
@epompeii
epompeii force-pushed the u/ep/benchmark-parameters/single-valued-metric branch from 244a3b3 to 53eb682 Compare August 23, 2026 05:22
@epompeii
epompeii force-pushed the u/ep/benchmark-parameters/single-valued-metric branch from 53eb682 to c6660d8 Compare August 23, 2026 06:34
@epompeii
epompeii marked this pull request as ready for review August 23, 2026 13:18
epompeii and others added 8 commits August 23, 2026 15:31
Tests first, red before green. The invariants pinned here are:

- Every response that reads a metric is byte identical across the migration.
  The bytes are captured on both sides of it, from the perf query, the metric
  endpoint, the report, and the alerts, over fixtures that carry both bounds,
  the lower bound only, the upper bound only, and neither, spread across two
  measures and two report benchmarks so that a pivot joining on the wrong key
  leaks a bound and fails. The floats are the ones that expose formatting
  drift rather than round numbers.
- The migration explodes each metric into its named rows, and the point
  estimate keeps the id and uuid of the row it came from, which is what makes
  the boundary remap a no-op.
- A down and up round trip leaves every response unchanged.
- Ingest writes the metric triple as named rows, and the report it answers
  with is the report the single row shape answered with.
- A bounded metric is one measurement, not three: the billable count does not
  move when the triple becomes three rows.

The schema, `MetricName`, and a stub migration ship here as the minimum that
lets the tests compile. The stub carries only the `value` row over and pivots
the bound columns to NULL, and ingest writes only the `value` row, so the
equivalence, explosion, and ingest tests fail on their assertions.

Reading a metric moves onto the `metric_boundary` view everywhere, including
the two alert queries that read the table directly. That is what lets one
binary serve both captures: the view's column list is what the migration holds
unchanged, so the same code reads either shape.

The parameter migration is no longer the last one, so its tests revert down to
it by version rather than reverting whatever happens to be on top.
`metric` is now `(report_benchmark_id, measure_id, name, value)`. The migration
explodes each old row into up to three: a `value` row that keeps the original id
and uuid, and a row for each stored bound under its conventional name. Keeping
the id is what makes the boundary remap a no-op, so no `boundary` row is
rewritten and its unique constraint is undisturbed. Pure SQL has no UUIDv7
function, so the new rows take a v4 minted from `randomblob`, the same recipe the
parameter migration uses.

`metric_boundary` pivots the named rows back into the columns its readers already
know. Holding its column list unchanged is what lets this layer claim response
equivalence without rewriting the readers: the perf query, the metric endpoint,
the report, and the alerts all see the shape they saw before. The self joins ride
`UNIQUE(report_benchmark_id, measure_id, name)`, and because that index makes
them at most one row, SQLite omits them outright for any query that does not
select a bound.

Ingest writes the point estimate first, so `last_insert_rowid` still names the
row a boundary attaches to, then the bounds. Detection has only ever gated the
value scalar, and it still does.

`QueryMetric::usage` counts the point estimate only. Named values collapse into
their measure's series, so a bounded metric bills as one measurement, exactly as
it did when it was one row.
`usage_does_not_count_the_bound_rows` seeds a metric with both of its bounds and
asserts that three rows still bill as one measurement. Removing the filter from
`QueryMetric::usage` fails it, which is the property that matters: the claim that
billing does not move is now held by a test rather than by a comment.

The integration test it replaces counted `value` rows without ever calling
`usage`, so it pinned nothing that the explosion test did not already pin, under
a name that promised more than it checked.
Two invariants of the single valued metric layer were unpinned.

Threshold detection reads its sample straight off the metric table filtered
by measure. Now that a bound is a named row under the same measure, that
sample mixes point estimates with previously computed limits, so the mean
and standard deviation it computes are taken over a mixture. Two projects
with the same point estimates now have to produce the same boundary,
whether or not their history carries bounds.

The migration mints uuids for the rows it creates. They have to be v7
shaped, with a millisecond prefix taken once for the whole migration, so
the new rows append to the right edge of the uuid index instead of
scattering across every page of it.

The rest is hardening the tests that already passed, against mutations they
should have caught and did not. The fixture is seeded with explicit,
non-contiguous ids, so a migration that renumbers the rows it carries over
is visible rather than accidentally right. The boundary is followed through
to the measurement it was created against, not merely observed to be
unwritten. The round trip is anchored against the pre-migration capture, so
a symmetric error cannot cancel itself out, and it now asserts the indexes,
the view's column list, and both integrity pragmas. A duplicate name under
one measurement is asserted to collide.
The threshold sample is taken off the metric table filtered by measure. A
bound is a named row under that same measure now, so the filter has to name
the point estimate too, or the sample becomes a mixture of measurements and
the limits previously computed from them, and a bounded sample size fills
with a third of the history it asked for.

The migration's minted uuids become v7 shaped. The 48 bit millisecond
prefix is computed once into a temporary table and cross joined into both
bound inserts, so every row it creates lands at one point on the right edge
of the uuid index rather than scattering across every page of it. The rest
of the uuid is `randomblob`, with the variant nibble drawn the way it
already was.
MetricName declares MAX_LEN, but is_valid_metric_name only checks that the
name is non-empty, so a 65 character name is currently valid and the
constant is decoration.

The test asserts what the constant claims: 64 characters is valid and 65 is
not. It fails until the check is written.
is_valid_metric_name moves from is_valid_non_empty to is_valid_len, the same
helper resource names, slugs, and user names already validate through.
MAX_LEN was already crate::MAX_LEN, so the bound the type declared is now the
bound it enforces, and a metric name is checked exactly like every other
bounded name in the crate.

No name in the workspace is affected. The conventional names are short, every
tool output fixture is short, and nothing anywhere reaches 64 characters.
The rebuild rewrites the whole metric table, since every metric row
explodes into a value row and its two bounds. Declared on the table, the
two unique keys are maintained across every one of those inserts, two
B-trees held dirty a page split at a time, and the value rows carry their
original uuids, which are v4, so each insert lands on a random page of the
uuid index.

Declaring the table without them and building them as named indexes once
the rows are in place makes each build one external sort. Measured over the
whole rebuild, that is 16 times fewer page reads and 29 times fewer page
writes. A sort is also indifferent to where its keys fall, so the v4 uuids
stop costing anything.

What is enforced does not move. A named unique index and a table constraint
are the same index to SQLite, so the pivot rides the same key and the view
plans the same way. The round trip test pins the two index names it finds
rather than the autoindexes it used to.
@epompeii
epompeii force-pushed the u/ep/benchmark-parameters/single-valued-metric branch from c6660d8 to a875c68 Compare August 23, 2026 15:31
@epompeii
epompeii merged commit 1c87518 into devel Aug 23, 2026
61 checks passed
@epompeii
epompeii deleted the u/ep/benchmark-parameters/single-valued-metric branch August 23, 2026 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant