bench: provision.sh, so a box that can run bench/ is reproducible (#505) - #524
Conversation
…mmandprompt#505) Every bench harness assumed an environment that existed only because someone built it by hand. The knowledge was spread across memories, a HANDOFF.md that is deliberately not in the repository, and issue comments, so a second machine -- or this one after a rebuild -- could not reproduce a number. This is the first half: the PostgreSQL builds, the contrib the suites need, and an audit that says whether a box can run bench/ at all. It does NOT install Citus, TimescaleDB, DuckDB or the ClickBench dataset, and says so in its own header rather than leaving that to be discovered. ## `check` is the load-bearing part It changes nothing, runs anywhere, and answers the question the issue actually asks: can this box reproduce a number? A provisioning script nobody can verify is how the undocumented box happened in the first place. It writes down what was written down nowhere: * **The naming convention is a contract.** pgNNa is an assert build for the matrix; pgNNn is NON-assert and is where every number in docs/benchmarks.md comes from; pgNN_san is clang with address and undefined sanitizers. commandprompt#504 had to teach the profiler to refuse an assert build because asserts put verify_compact_attribute at 9.28% of an ingest profile. * **The suffix is checked against `pg_config --configure`, not trusted.** The name is a claim; the flags are the fact. A pg18n accidentally built with cassert would produce quietly wrong benchmark numbers and nothing else on the box would notice. * **The privilege split**, which fails in the worst direction: the installing harnesses redirect make install to /dev/null, so run unprivileged they fail QUIETLY and measure the previously installed .so. `check` warns when it is running as root, because run_bench_join.sh and run_clickbench.sh must not be. * **initdb needs --locale=C explicitly**, because sudo and runuser do not carry LANG and initdb then leaves a directory that is not a cluster. ## What it found btree_gist is present on all five ASSERT builds of the bench host and **absent from pg18n and pg18_san** -- the two prefixes that are not assert builds, which are exactly the two nobody had enumerated. pg18n is where every published benchmark number comes from. Since commandprompt#448, test/temporal.sh hard-fails rather than skipping when btree_gist is missing. ## Three traps this hit while being written **`make install PG_CONFIG=...` inside a configured source tree ignores PG_CONFIG.** A contrib directory there builds in-tree and installs to the parent tree's ./configure --prefix. The first version reported "installed btree_gist into pg18n" while the files went to an unrelated prefix -- exit 0, empty log, nothing where it was wanted. USE_PGXS=1 makes PG_CONFIG authoritative. (An out-of-tree extension like this one is unaffected: its Makefile takes PG_CONFIG as authoritative already.) **It trusted the exit status.** A build system that installs the right files into the wrong prefix returns 0, so the installer now verifies the .control file exists at `pg_config --sharedir` afterwards and fails if it does not. That verification is what caught the third trap. **A sanitizer build's own pg_config is instrumented and leaks on exit**, so with leak detection on it exits NON-ZERO and every `$(pg_config --sharedir)` comes back empty -- the failure then reads as "the extension is missing" rather than "I could not ask where it goes". detect_leaks=0 is exported for that reason. ## And the auditor had the same blind spot it was written to catch The first version enumerated the benchmark and matrix prefixes and skipped the sanitizer one, so it could not have reported the pg18_san gap -- the same "checked five, missed the sixth" shape that left the gap in the first place. The contrib audit and installer now iterate every prefix the script knows about. ## Tested by running it on two boxes that disagree An audit that cannot report a problem is worth nothing, so it was run where the answers differ. Every state it can emit -- ok, WARN, MISSING -- is produced by a real machine, and the root-user warning fires on one and not the other: bench host 19 ok, 0 warn, 1 missing (found the pg18n gap; read-only) container 13 ok, 4 warn, 4 missing before container 18 ok, 4 warn, 0 missing after, all seven prefixes `contrib` is idempotent: a second run reports "already in" and writes nothing. Refs commandprompt#505, commandprompt#504, commandprompt#448. ## Getting the source: a tarball is enough, and that is the safer shape `contrib` no longer needs a configured source tree. It resolves an extracted tree if one exists, otherwise extracts `postgresql-<version>.tar.*` to a scratch directory, and always builds with USE_PGXS=1 against the target prefix's own installed pgxs. That is jdatcmd's suggestion from doing it by hand on the bench, where no extracted tree existed at all, and it is better than what I had: it sidesteps the in-tree PG_CONFIG trap **by construction** instead of guarding against it. `check` now also reports whether a missing module is fixable on this box: MISSING btree_gist in pg16a (test/temporal.sh hard-fails; 'provision.sh contrib' can fix it) MISSING btree_gist in pg16a AND no contrib source for PostgreSQL 16.14 on this box Reporting a problem without saying whether it can be fixed here is half an answer, and the half that sends someone looking. Both branches were exercised by removing btree_gist from pg16a and re-running with and without a reachable source.
jdatcmd
left a comment
There was a problem hiding this comment.
Approved, and run against the bench rather than reviewed only as a diff — which is the point of having a check mode, so I used it:
21 ok, 0 warn, 0 missing
and the tree was clean afterwards, so read-only holds in practice and not just by intent.
The prediction I left, and how it came out
I said this should report 20 ok / 0 warn / 0 missing on the bench once I had installed btree_gist into pg18n and pg18_san. The testable half held — 0 missing — and the count came out 21 because you added the seventh-prefix check between my prediction and the run. The box matched; the instrument had gained a question.
Worth naming, because it is a small version of something that has bitten us both today: a predicted count is only falsifiable against a fixed instrument. Had I treated 21-against-20 as a disagreement I would have gone looking for a missing module that was never missing.
The design decisions I would single out
USE_PGXS=1 sidesteps the in-tree trap by construction rather than guarding against it, which is the stronger form and the one I hoped you would take. A guard has to be maintained; a construction that cannot express the bug does not.
ASAN_OPTIONS/LSAN_OPTIONS exported with detect_leaks=0 defaults is the fix for a failure that reads as something else entirely. I reproduced it before relying on it: pg18_san's pg_config --sharedir returns rc 1 bare and rc 0 with the option, so the empty $(...) really does turn "I could not ask where it goes" into "the extension is missing". Two different problems that look identical in a log.
Verifying the .control at $(pg_config --sharedir) after installing, rather than trusting rc=0, is what surfaced that trap at all. It is the same rule as reading the ASAN report instead of counting the files, and as printing a check count beside a fail count: the attempt succeeding is not the outcome happening.
Stating in the header what it does NOT do — Citus, TimescaleDB, DuckDB, ClickBench — is what stops it becoming a lie in three months. And the reasoning for excluding Citus and Timescale is right: both must be built against one exact PostgreSQL, and choosing that silently is how a comparison ends up measuring the wrong engine.
The finding behind it is the sharpest of the day
Your auditor's contrib loop skipped pg18_san — the tool written to catch "checked five, missed the sixth" had that exact shape encoded in its own loop. I made the same generalisation in a sentence this morning; yours was in a mechanism, and a mechanism repeats. That it took a seventh prefix to surface a blind spot in the thing built to find blind spots is worth keeping in the script's header, not just in this thread.
One gap you already named and I would leave open rather than fix here: check reports that a module is missing but not whether it can fix it — it does not verify a source tree exists. On the bench there was only a tarball, and USE_PGXS=1 meant no configured tree was needed at all, so extracting to a scratch dir was enough. That may be the general answer rather than requiring one tree per prefix.
Merging.
Closes #505.
There was no way to stand up a machine that can run
bench/. Every harness assumed an environment that existed only because someone built it by hand, and the knowledge was spread across memories, aHANDOFF.mddeliberately not in the repository, and issue comments. A second machine — or this one after a rebuild — could not reproduce a number.Scope, stated in the script's own header rather than left to be discovered: this does the PostgreSQL builds, the contrib the suites need, and an audit. It does not install Citus, TimescaleDB, DuckDB or the ClickBench dataset. Citus and TimescaleDB must be built against one exact PostgreSQL, and choosing that silently is how a comparison ends up measuring the wrong engine.
checkis the load-bearing partIt changes nothing, runs anywhere, and answers the question the issue actually asks: can this box reproduce a number? It also writes down what was written down nowhere —
pgNNaassert (matrix),pgNNnnon-assert (every number in docs/benchmarks.md),pgNN_sanclang + ASan/UBSan. fix: a profile must not measure the assert build or its own fixture (#445) #504 had to teach the profiler to refuse an assert build, because asserts putverify_compact_attributeat 9.28% of an ingest profile.pg_config --configure, not trusted. The name is a claim; the flags are the fact.make installto/dev/null, so run unprivileged they fail quietly and measure the previously installed.so.initdbneeds--locale=C, because sudo and runuser dropLANG.What it found
btree_gistwas present on all five assert builds of the bench host and absent frompg18nandpg18_san— the two prefixes that are not assert builds, which are exactly the two nobody had enumerated.pg18nis where every published benchmark number comes from, and since #448test/temporal.shhard-fails rather than skipping without it.@jdatcmd has since installed it on both bench prefixes and confirmed
temporalnow runs there (5 checks, 0 fails onpg18n; the other three are PG19-only).Four traps hit while writing it
make install PG_CONFIG=...inside a configured source tree ignoresPG_CONFIG. A contrib directory there builds in-tree and installs to the parent tree's--prefix. The first version reported "installed btree_gist into pg18n" while the files went elsewhere — exit 0, empty log, nothing where it was wanted..controlfile exists atpg_config --sharedirafterwards — and that verification is what caught the third trap.pg_configis instrumented and leaks on exit, so with leak detection on it exits non-zero and every$(pg_config --sharedir)comes back empty. The failure then reads as "the extension is missing" rather than "I could not ask where it goes".detect_leaks=0is exported for that reason.pg18_sangap — the same "checked five, missed the sixth" shape that left the gap. It now iterates every prefix it knows about.Getting the source
contribdoes not need a configured tree: it resolves an extracted one, else extractspostgresql-<version>.tar.*to a scratch dir, and always builds withUSE_PGXS=1against the target prefix's own pgxs. That sidesteps trap 1 by construction rather than guarding against it (@jdatcmd's shape, from doing it by hand on a bench with no extracted tree).checkalso says whether a missing module is fixable here, because reporting a problem without that is the half that sends someone looking:Tested on two boxes that disagree
An audit that cannot report a problem is worth nothing, so every state it emits is produced by a real machine, and the root-user warning fires on one and not the other:
Both
contribbranches were exercised by removingbtree_gistfrompg16aand re-running with and without a reachable source.contribis idempotent — a second run reports "already in" and writes nothing.Gate
harness_selftest54/0,docs_style6/0,bench_guards17/0, 0 warnings, and the full matrix on PG18 + PG19 ALL VERSIONS PASSED. No suite executes or readsbench/provision.sh(bench_guardsinspectscb_guards.shonly), so the matrix outcome cannot turn on it — it was run anyway.