Repair report, version, and branch indexes and remove orphaned rows - #922
Merged
Conversation
Contributor
|
| Project | Bencher |
| Branch | reindex-report-version |
| Testbed | intel-v1 |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result microseconds (µs) (Result Δ%) | Upper Boundary microseconds (µs) (Limit %) |
|---|---|---|---|
| Adapter::Json | 📈 view plot 🚷 view threshold | 4.61 µs(-1.61%)Baseline: 4.69 µs | 4.99 µs (92.48%) |
| Adapter::Magic (JSON) | 📈 view plot 🚷 view threshold | 4.54 µs(-0.36%)Baseline: 4.55 µs | 4.81 µs (94.23%) |
| Adapter::Magic (Rust) | 📈 view plot 🚷 view threshold | 26.15 µs(+1.25%)Baseline: 25.83 µs | 27.11 µs (96.46%) |
| Adapter::Rust | 📈 view plot 🚷 view threshold | 3.45 µs(-2.29%)Baseline: 3.53 µs | 3.93 µs (87.68%) |
| Adapter::RustBench | 📈 view plot 🚷 view threshold | 3.45 µs(-2.29%)Baseline: 3.53 µs | 3.94 µs (87.62%) |
epompeii
force-pushed
the
reindex-report-version
branch
from
August 23, 2026 00:10
3acb9da to
95e0998
Compare
Add a migration that rebuilds all indexes on the report, version, and branch tables. Long-lived databases can carry index corruption (wrong number of entries in an index) that silently drops rows from index-driven queries. The corruption found in July reached the report indexes, the version unique index, and index_branch_head, so all three tables are rebuilt. REINDEX is a no-op when the indexes are already consistent, so this is safe for every instance. The migration also removes report_benchmark rows (and their dependent metric, boundary, and alert rows) orphaned by deletes performed before ON DELETE CASCADE was enforced (see 2023-01-15-185835_perf_cascade). Migrations run with foreign_keys = OFF, so the delete chain is explicit and bottom-up, with the orphaned ids collected into temp tables up front so every delete works from one stable set. The Litestream startup cleanup this branch used to carry is dropped by decision. Standing deletion code in the data directory is not something to add here; a detect-and-log variant may return separately.
epompeii
force-pushed
the
reindex-report-version
branch
from
August 23, 2026 01:01
95e0998 to
2dadf83
Compare
epompeii
marked this pull request as ready for review
August 23, 2026 01:43
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.
Migration:
reindex_report_versionLong-lived databases can carry index corruption (
wrong # of entries in index) that silently drops rows from index-driven queries, while the underlying table data remains intact. This migration rebuilds all indexes on thereport,version, andbranchtables.REINDEXis a no-op when the indexes are already consistent, so this is safe for every instance.branchis new to the list since the first draft of this pull request. The corruption found in July reached thereportindexes,sqlite_autoindex_version_1, andindex_branch_head.index_branch_headhangs off thebranchtable, so neitherREINDEX reportnorREINDEX versionever touched it. Naming the table rather than the index rebuilds every index on it, including the implicitUNIQUEautoindexes: 9 indexes onreport, 1 onversion, 5 onbranch.branchis a small table next to the ones it sits with, so the added cost is nil.The migration also removes
report_benchmarkrows orphaned by deletes performed beforeON DELETE CASCADEwas enforced (see2023-01-15-185835_perf_cascade), along with their dependentmetric,boundary, andalertrows. Migrations run withforeign_keys = OFF, so cascades do not fire and the delete chain is explicit, bottom-up. The orphaned ids are collected into temp tables up front so that every delete works from one stable set, rather than each statement re-running the same multi-table join over tables an earlier delete has already changed.Sequencing
This migration runs immediately before
2026-08-15-120000_benchmark_parameter. That migration rebuildsreport_benchmarkwith aNOT NULL parameter_idfilled through aLEFT JOINon the backfilledparametertable, so areport_benchmarkrow whose benchmark no longer exists trips theNOT NULLand aborts the migration instead of being dropped silently. The rows this migration removes are exactly the rows that trip it. Diesel selects pending migrations as every embedded version not yet recorded, sorted ascending by version, so2026-07-06-120000runs first even on a database already past2026-07-18-120000.Validation
Validated against a production-shaped copy, taken copy-on-write so the source is never touched, with
PRAGMA integrity_checkclean before the run.The copy carries a handful of orphaned
report_benchmarkrows, every one of them hanging off the same missing benchmark, each with one dependentmetricrow and noboundaryoralertrows.After the migration:
report_benchmarkandmetriceach lose exactly those orphaned rows and nothing elseboundary,alert,benchmark,report,version,branch, andseries_last_seenare unchanged, row for rowreport_benchmarkrows remainPRAGMA foreign_key_checkreturns no rowsPRAGMA integrity_checkreturnsokindex_branch_headis still present, and the temp tables leave nothing behind in the schemaThe three benchmark-parameters migrations then apply on top of the repaired copy.
2026-08-15-120000_benchmark_parameterbackfills oneparameterrow per benchmark, leaves noreport_benchmarkrow with a nullparameter_id, and holdsreport_benchmarkat the repaired count.Run against the same copy with this migration skipped,
2026-08-15-120000_benchmark_parameterinstead fails withNOT NULL constraint failed: up_report_benchmark.parameter_idand rolls back. That abort is what this migration removes.One derived value does go stale.
metric_count_by_reportcaches a metric count per report and is not decremented here, so each affected report keeps a count one higher than the metrics it still has. That rollup feeds server stats rather than the ingest meter, anddelete_report_resultsalready accepts letting it drift. The precedent is not the same shape, though: there the drift is transient, since the rollup row cascades away when the report row is deleted, while these persist.cargo nextest run -p bencher_schema --all-featurespasses 182 tests. Every one of them builds its database by running the full migration chain, so these statements are proven to apply under diesel inside a transaction.Not in this pull request
An earlier version of this branch also removed stale Litestream files at API server startup: the
generations/state from Litestream 0.3 that 0.5+ no longer reads, and leftover<db>.tmp*files from an interrupted restore. That half is dropped by decision. Standing deletion code aimed at the data directory is a heavy way to reclaim dead bytes, and a variant that only detects and logs them may come back separately.