perf(api): Stop replaying migrations, and give tests their own database - #8378
Draft
khvn26 wants to merge 3 commits into
Draft
perf(api): Stop replaying migrations, and give tests their own database#8378khvn26 wants to merge 3 commits into
khvn26 wants to merge 3 commits into
Conversation
4 tasks
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8378 +/- ##
==========================================
- Coverage 98.80% 98.64% -0.17%
==========================================
Files 1619 1622 +3
Lines 65682 66082 +400
==========================================
+ Hits 64898 65187 +289
- Misses 784 895 +111 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The API suite took a little over twenty minutes, and a single test took around twenty-five seconds. Both were dominated by work that had nothing to do with the tests being run. Nearly all of it was migration replay. Creating a test database runs the whole ~600 migration history, and pytest-django does it once per xdist worker, so a cold ten-worker run paid for it ten times. Every migration test then replayed the history from zero to reach the state it tests, and migrated all the way forward again on teardown -- the 128 of them accounted for half the suite's CPU on their own. ClickHouse was the same story from a different angle: Django replayed all ~600 migrations against it to build the migration state for the three it owns, twenty-two seconds of it, on every session -- including a run of one Postgres-only test. PostgreSQL copies a database in about a tenth of a second, so cache each migration state as a template database and clone it instead. States are keyed on the length of the migration plan prefix they correspond to, which makes them nest: reaching a deeper state clones the nearest cached ancestor and replays only the migrations in between, so a session converges on the cost of one migration run however many migration tests it has. Template names embed a digest of the migration files, so editing a migration -- or switching branches -- builds a new template rather than handing anyone a stale schema. That is `--reuse-db` without the footgun, and it makes the `--ci` database bootstrap unnecessary. Test data is throwaway, which makes a Docker volume the wrong place for it: on macOS its I/O is virtualised, and it made the suite's runtime a function of how busy the host's disk was. Tests now run against `test-db`, a separate RAM-backed server tuned for a workload that clones databases and never needs to survive a crash. The rest is smaller change of the same character: the default password hasher costs 350ms a call and fixtures hash on almost every test; coverage roughly doubles the runtime and is now `make test-coverage`; `-vvvv` spent seconds formatting a line per test; and `make test` recreated every container and booted Django twice just to check the databases were up. Suite: ~1240s to ~150s on an idle machine. Individual runs: p95 6.7s over 40 sampled test files, against ~25s of fixed cost before a test even ran.
Setting up the databases, Django serialises every model in every one of
them so that `TransactionTestCase(serialized_rollback=True)` can restore
them later. Nothing in this suite asks for that, so the work is pure cost.
On the ClickHouse alias it is worse than pure cost. Serialising builds a
`MigrationLoader`, and `django-clickhouse-backend` caches its migration
model on `MigrationRecorder` -- which carries a `deleted` column that
Django's does not. Get there with a PostgreSQL connection first and the
cache holds a model without that column, while the queryset still filters
on it, so the ClickHouse alias fails with
FieldError: Cannot resolve keyword 'deleted' into field
That only surfaces in the private build, where an app routed to ClickHouse
gives the alias something to serialise.
khvn26
force-pushed
the
perf/api-test-suite
branch
from
August 26, 2026 12:22
d5c4d29 to
e50dbd7
Compare
The cache drops templates whose migration graph no longer exists, which is what stops every branch a working copy visits leaving a copy of the database behind. CI never exercises it -- a job only ever sees one graph -- so assert it directly.
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.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Creating a test database runs the whole ~600 migration history, and pytest-django does it once per xdist worker, so a cold ten-worker run paid for it ten times. Every migration test then replayed the history from zero to reach the state it tests, and migrated forward again on teardown — the 128 of them accounted for half the suite's CPU on their own, from 2.6% of the tests. ClickHouse was the same story from a different angle: Django replays all ~600 migrations against it to build the migration state for the three tables it owns, 22 seconds of it, on every session — including a run of one Postgres-only test. Profiling that showed 28s in
render_multipleand no SQL statement over 0.2ms.PostgreSQL copies a database in about a tenth of a second, so
api/tests/migration_snapshots.pycaches each migration state as a template database and clones it instead. States are keyed on the length of the migration plan prefix they correspond to, which makes them nest: reaching a deeper state clones the nearest cached ancestor and replays only the migrations in between, so a session converges on the cost of one migration run however many migration tests it has. Template names embed a digest of the migration files, so editing a migration — or switching to a branch with different ones — builds a new template rather than handing anyone a stale schema. That is--reuse-dbwithout the footgun, and it makes the--cidatabase bootstrap unnecessary. ClickHouse gets the same treatment through the copy its backend already knows how to do.Test data is throwaway, which makes a Docker volume the wrong place for it: on macOS its I/O is virtualised, and it made the runtime a function of how busy the host's disk was — 258s against 137s on identical code. Tests now run against
test-db, a separate RAM-backed server tuned for a workload that clones databases and never needs to survive a crash. Nothing in it is worth keeping, sodocker compose restart test-dbis always a safe reset, and an empty server just rebuilds its templates. Thedbservicemake serveuses is left alone, with its durability intact.The rest is smaller change of the same character:
admin_userhash on nearly every test. Tests use the cheapest hasher instead; nothing depends on the algorithm.make test-coverage, which CI uses.-vvvvinaddoptsspent seconds formatting a line per test — 8s on collection alone — and buried failures in the noise.make testrecreated every container and booted Django twice just to check the databases were up, about 12s before pytest started. It now probes the sockets and only calls Compose when something is actually down.make teststill fans out with xdist;make test opts=...is taken to be a focused run and stays in-process, because each worker costs a Python start, a Django setup and a database clone.4937 passed, 23 skipped, unchanged from before.One thing this surfaces
Three
enqueue_membership_refreshtests intests/unit/segment_membershipare latently flaky, roughly one run in five.get_tasks_to_processfilters onscheduled_for < NOW(), butdelay()stampsscheduled_forfrom the client's clock, and the test database's clock runs about 0.08ms behind the host's — so a task enqueued and polled in the same breath is briefly invisible to the poller. The suite used to be slow enough that this never mattered. Nothing here causes it and I have deliberately not papered over it; the durable fix belongs in flagsmith-common, where the database should stampscheduled_forrather than the client.Left on the table
Each ClickHouse test currently drags Postgres down to
TransactionTestCasesemantics, because Django only wraps a test in transactions when every database it declares supports them. That costs a full flush and apost_migraterebuild of every permission, about 2.5s a test and ~60s of the suite.fake_transactionon the ClickHouse alias fixes it, but it also changeson_commitbehaviour and breaks tests that count queued tasks, so it is a behaviour change rather than an optimisation and wants its own PR.How did you test this code?
The suite is the test: same 4937 passed, 23 skipped as before, over six consecutive full runs plus several from a cold (empty) test server, which exercises the template-building path rather than the clone path.
To check the caching is honest rather than just fast, migration tests were run both with templates already built and against a freshly restarted
test-db, and-k migratwas confirmed to still select all 128 tests — themigrator_factoryoverride keeps the fixture's name sodjango_test_migrationsstill marks them.Timings were taken on an otherwise idle machine, against
origin/mainas the control under the same conditions. Focused-run figures are the wall time ofmake test opts='<file>'over 40 randomly sampled test files, and separately over 20 hand-picked selectors chosen to include the awkward cases (a single test, a migration test, a ClickHouse-heavy directory, a bare-kexpression).To sanity-check the new database: