test(coderd/database/dbtestutil): disable Postgres JIT for test databases - #29465
Open
ibetitsmike wants to merge 1 commit into
Open
ibetitsmike wants to merge 1 commit into
ibetitsmike wants to merge 1 commit into
Conversation
…ases On near-empty test databases the planner misestimates some queries so badly that they exceed jit_above_cost, and Postgres LLVM-compiles them on every call. GetUserStatusCounts alone spent about 275ms of its 290ms execution in JIT. JIT is an execution optimization with no semantic effect, so disable it per test database next to the existing timezone setting.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
On near-empty test databases the planner misestimates some queries so badly that they exceed
jit_above_cost, and Postgres then LLVM-compiles them on every call. ForGetUserStatusCountsthat is about 275ms of a 290ms execution (EXPLAIN (ANALYZE)on an empty database estimates 5.7M rows for thedates_of_interest x relevant_status_changesnested loop; actual rows: 61). Multiplied by the 925 leaves ofTestGetUserStatusCounts, that single test was 85% of thecoderd/databasepackage time.JIT is an execution optimization with no semantic effect, so
dbtestutil.NewDBnow runsALTER DATABASE <db> SET jit = offon each per-test database, next to the existing timezone setting. Database-level settings are not copied byCREATE DATABASE ... TEMPLATE, so this has to run per clone, which is where the timezone statement already runs; theCODER_PG_CONNECTION_URLpath goes through the same function.Before / After
Local,
go test -count=1 -tags=testsmallbatch -parallel=8(shared host, load about 50):./coderd/database/ -run '^TestGetUserStatusCounts$'ok 63.2sok 6.4s./coderd/database/ok 57.2sok 10.0s./coderd/database/dbauthz/ok 7.4s/14.0sok 10.3s/7.6s(noise)./coderd/database/dbpurge/ok 4.1sok 3.8s./coderd/httpmw/ok 1.3sok 1.0sOnly
coderd/database(172s in the baseline CI job) is expected to move materially; the other packages are listed to show no regression.Validation
TestGetUserStatusCountsstill runs its 925 subtestsgolangci-lint run ./coderd/database/dbtestutil/...with a fresh cache: 0 issuesProduction note, not addressed here: with
jit=on(the default in the pgdg builds) production pays the same per-call compilation forGetUserStatusCounts; the query shape (generate_seriescross join with no statistics on the CTE) deserves its own fix.Part of the test-suite speedup series (see #29457). Independent of the other PRs; #29464 (fixture sharing in the same test) composes with it.
Remote validation
Remote run on a dogfood workspace (separate compute), chat https://dogfood.cdr.dev/agents/5bbe73d0-866a-4841-9cc3-c5e5b4d97e44, tested head
353879f5d12against base9d973372774on the same machine (its base time, 56.4s for the package, matches the table above):./coderd/database/ -run '^TestGetUserStatusCounts$'46.97s to 13.81s (925 subtests pass on both);./coderd/database/56.43s to 20.49s (32.48s on a first run);dbauthz,dbpurge,httpmwunchanged;-race -count=2 -shuffle=on: pass. A livepg_db_role_settingquery during a test run showed{TimeZone=...,jit=off}on every per-test database and nothing on thetpl_template, as intended. The standalone speedup there is 3.4x for the test and about 2x for the package, less than the 10x and 5.7x in the table above, which came from a single run on the development host: on the dogfood host the per-test database clone dominates once JIT is off. The headline numbers (test 0.6s, package 7.5s) are reached together with #29464, which removes the per-leaf clone. Endorsed verdict: PASS on behavior; the table above overstates the standalone gain.