Skip to content

Commit 9f7d5ea

Browse files
ci(migrations): Cache the migrated test databases between runs (#128394)
`backend / backend migration tests` spends about 237s on master CI building `test_region`, `test_control` and `test_secondary`. The first test's setup runs every migration from scratch. With this change: - On master, after the tests pass, the job saves a `pg_dump` of the three test databases plus a manifest of the migration files it was built from. The cache key hashes `src/**/migrations/*.py` and `src/sentry/new_migrations/**`. - Every run restores the exact key, or the newest master dump through `restore-keys`, before running pytest. The job already uses `--reuse-db`, so pytest-django keeps the restored databases and applies only the migrations added since the dump. - If the manifest lists a migration file this checkout doesn't have, the script skips the restore and pytest builds the databases from scratch as it does today. That happens when a PR branch is behind the master commit that saved the dump. - `.github/file-filters.yml`: changes to the new script now trigger the migration tests. **Local measurements (single migration test, `--migrations --reuse-db`)** - From scratch: 306s DB setup - After restoring a dump (~6s): ~40s DB setup; the test passes - I also checked that a manifest with an unknown migration skips the restore, and that a missing cache falls through to a normal build. **Not verified yet** - No cache exists until this lands, so this PR's own run takes the "no saved databases" path. The first master run saves the dump, and the run after that is the first real restore in CI. - I didn't check the case where the dump is behind and pytest applies the newer migrations on top. That path is plain pytest-django `keepdb` plus `migrate`. <!-- junior-request-attribution:start --> via @joshuarli. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3ACUHS29QJ0%3A1792602543.346169 --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/slack%3ACUHS29QJ0%3A1792602543.346169) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3ACUHS29QJ0%3A1792602543.346169/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Joshua Li <14209781+joshuarli@users.noreply.github.com>
1 parent bda7d4d commit 9f7d5ea

3 files changed

Lines changed: 99 additions & 0 deletions

File tree

‎.github/file-filters.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ migrations_added:
175175
# Migration tests only exercise migrations, so PRs only run them when these change.
176176
migration_tests:
177177
- '.github/workflows/backend.yml'
178+
- '.github/workflows/scripts/migration-test-db.py'
178179
- 'Makefile'
179180
- 'src/**/migrations/**'
180181
- 'tests/**/migrations/**'

‎.github/workflows/backend.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,9 @@ jobs:
369369
contents: read
370370
actions: read
371371
pull-requests: write
372+
env:
373+
MIGRATION_DB_DIR: .artifacts/migration-test-db
374+
SAVE_MIGRATION_DB: ${{ github.ref == 'refs/heads/master' }}
372375

373376
steps:
374377
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
@@ -379,10 +382,33 @@ jobs:
379382
with:
380383
mode: default
381384

385+
# Only master saves, so PRs restore the newest master dump.
386+
- name: Restore migrated test databases
387+
id: db-cache
388+
uses: actions/cache/restore@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
389+
with:
390+
path: ${{ env.MIGRATION_DB_DIR }}
391+
key: migration-test-db-v1-${{ hashFiles('src/**/migrations/*.py', 'src/sentry/new_migrations/**/*.py') }}
392+
restore-keys: migration-test-db-v1-
393+
394+
- name: Load migrated test databases
395+
run: .github/workflows/scripts/migration-test-db.py restore "$MIGRATION_DB_DIR"
396+
382397
- name: run tests
383398
run: |
384399
make test-migrations-ci
385400
401+
- name: Dump migrated test databases
402+
if: env.SAVE_MIGRATION_DB == 'true' && steps.db-cache.outputs.cache-hit != 'true'
403+
run: .github/workflows/scripts/migration-test-db.py save "$MIGRATION_DB_DIR"
404+
405+
- name: Save migrated test databases
406+
if: env.SAVE_MIGRATION_DB == 'true' && steps.db-cache.outputs.cache-hit != 'true'
407+
uses: actions/cache/save@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
408+
with:
409+
path: ${{ env.MIGRATION_DB_DIR }}
410+
key: ${{ steps.db-cache.outputs.cache-primary-key }}
411+
386412
- name: Report failures
387413
if: ${{ !cancelled() && github.event_name == 'pull_request' }}
388414
continue-on-error: true
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
#!/usr/bin/env python3
2+
"""Save and restore the migrated test databases used by the migration tests.
3+
4+
With --reuse-db, pytest keeps a restored database and only applies the
5+
migrations added since it was dumped, instead of running every migration.
6+
7+
Usage: migration-test-db.py save|restore DIR
8+
"""
9+
10+
from __future__ import annotations
11+
12+
import subprocess
13+
import sys
14+
from pathlib import Path
15+
from typing import BinaryIO
16+
17+
POSTGRES_CONTAINER = "postgres-postgres-1"
18+
POSTGRES_USER = "postgres"
19+
# Django's test names for the region, control and secondary databases.
20+
TEST_DATABASES = ("test_region", "test_control", "test_secondary")
21+
MIGRATION_GLOB = "src/**/migrations/*.py"
22+
MANIFEST = "migrations.txt"
23+
24+
REPO_ROOT = Path(__file__).resolve().parents[3]
25+
26+
27+
def migration_files() -> set[str]:
28+
return {str(p.relative_to(REPO_ROOT)) for p in REPO_ROOT.glob(MIGRATION_GLOB)}
29+
30+
31+
def postgres(*args: str, stdin: BinaryIO | None = None, stdout: BinaryIO | None = None) -> None:
32+
subprocess.run(
33+
["docker", "exec", "-i", POSTGRES_CONTAINER, *args], check=True, stdin=stdin, stdout=stdout
34+
)
35+
36+
37+
def save(cache_dir: Path) -> None:
38+
cache_dir.mkdir(parents=True, exist_ok=True)
39+
for db in TEST_DATABASES:
40+
with open(cache_dir / f"{db}.dump", "wb") as f:
41+
postgres("pg_dump", "-U", POSTGRES_USER, "--format=custom", db, stdout=f)
42+
(cache_dir / MANIFEST).write_text("\n".join(sorted(migration_files())))
43+
44+
45+
def restore(cache_dir: Path) -> None:
46+
manifest = cache_dir / MANIFEST
47+
if not manifest.is_file():
48+
print("No saved databases; pytest will build them.")
49+
return
50+
51+
# A dump with migrations this checkout lacks (e.g. a branch behind master)
52+
# can't be migrated forward, so build from scratch instead.
53+
missing = set(manifest.read_text().split()) - migration_files()
54+
if missing:
55+
print(f"Saved databases include {len(missing)} unknown migrations; pytest will build them.")
56+
return
57+
58+
for db in TEST_DATABASES:
59+
postgres("createdb", "-U", POSTGRES_USER, db)
60+
with open(cache_dir / f"{db}.dump", "rb") as f:
61+
postgres("pg_restore", "-U", POSTGRES_USER, "--dbname", db, stdin=f)
62+
63+
64+
def main() -> None:
65+
commands = {"save": save, "restore": restore}
66+
if len(sys.argv) != 3 or sys.argv[1] not in commands:
67+
sys.exit(__doc__)
68+
commands[sys.argv[1]](Path(sys.argv[2]))
69+
70+
71+
if __name__ == "__main__":
72+
main()

0 commit comments

Comments
 (0)