Conversation
Add a `postgres` instrument enabled via `--instruments postgres`. Unlike the MongoDB instrument it owns no process and does no proxying: the `codspeed/postgres` image runs a poller that writes a pg_stat_statements analytics dump to a file. This instrument copies that dump (path given by the required `--postgres-dump-path`) into `<profile_folder>/instruments/postgres.json` after the run, so it rides along in the uploaded profile archive. collect() waits for the poller to flush a dump newer than the run end before copying, so the final queries aren't dropped by the poller's tick interval.
Merging this PR will not alter performance
|
Replace the whole-run poller-copy instrument with per-benchmark capture
driven by the runner. On its own connection (--postgres-dsn), reset
pg_stat_statements at each benchmark's StartProfiler and snapshot + EXPLAIN
at StopProfiler, in the pre-Ack window so the SQL stays outside the measured
region. Key each snapshot to the benchmark URI (the same one its flamegraph
uses) and write a per-benchmark artifact instruments/postgres.json =
{benchmarks:[{uri, queries}]}.
Drops --postgres-dump-path and the whole-run collect; the image now only
needs pg_stat_statements preloaded, and the read+EXPLAIN logic lives in the
runner.
Take --postgres-dsn-env-name (the name of an env var holding the DSN) instead of the DSN itself, mirroring --mongo-uri-env-name. The DSN is resolved at connect time and never stored in the config, so it can't leak into the config debug dump, runner.log, or the uploaded archive, nor onto the command line. Also skip the artifact entirely (rather than zip by index) when the URI and snapshot counts disagree, so queries are never misattributed to the wrong benchmark, and keep a DSN-gated isolation test covering the reset-per-boundary invariant.
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.
Add a
postgresinstrument that captures each benchmark's SQL, call counts, buffer hits, and query plans frompg_stat_statements, keyed to that benchmark's flamegraph.Enabled with
--instruments postgres --postgres-dsn-env-name <ENV>, where the env var holds a superuser connection string — passed by name, not value, so the DSN never reaches the command line, the config dump, or the uploaded logs (mirroring--mongo-uri-env-name).The runner observes benchmark boundaries over the instrument-hooks FIFO and, on its own connection, resets
pg_stat_statementsat each benchmark's start and snapshots +EXPLAIN (GENERIC_PLAN, FORMAT JSON)s at each stop — both in the pre-Ack window, so the SQL runs outside the measured region and never perturbs the walltime/flamegraph measurement. Each snapshot is keyed to the benchmark URI (the same one its flamegraph uses) and written to<profile_folder>/instruments/postgres.jsonas{ benchmarks: [{ uri, queries: [...] }] }, riding along in the uploaded profile archive.Capture is non-fatal: a connection or query failure is logged and never fails the run. On a URI/snapshot count mismatch the artifact is skipped rather than risk misattributing queries to the wrong benchmark.
Requires Postgres 16+ (for
GENERIC_PLAN) and a superuser role; targets walltime runs for now (simulation has no runner-side boundary callback yet).