fix(tests): run the maintenance-routine e2e against the current routine body - #3680
Merged
nicoloboschi merged 1 commit intoAug 21, 2026
Merged
Conversation
…ne body `test_routines_callable_from_non_public_schema` installs the routines into a non-public schema and then CALLS one, but it loaded the bodies from `b6d2f8a4c1e7` — superseded since. #2638's install gating is what the test is about and that has been carried forward unchanged, but the bodies have not: `c8b4e2a71f95` gave each per-schema query a `lock_timeout` and skip arms precisely because the older body waits indefinitely for AccessShareLock on every schema it scans, and a peer's concurrent DROP then closes a lock cycle. So the test kept exercising the one body no deployment runs, and under xdist — where peer workers create and drop schemas continuously — PostgreSQL kept picking it as the deadlock victim. Recurring `DeadlockDetectedError`, most recently on main's own run 32142821174. Measured with the victim schema held under ACCESS EXCLUSIVE from a second connection: the b6d2f8a4c1e7 body blocks until the DDL commits, the current one returns immediately having skipped it. Resolve the migration from the revision chain (Alembic's `walk_revisions`, so merge revisions resolve correctly) rather than hard-coding it, and add a guard asserting whichever migration owns the bodies still carries the skip arms — the point being that a fix to the routine must reach the test that reports the bug. `_load_schema_local_migration` stays pinned for the two tests that assert what that migration's install gating emits.
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.
The flake
test_maintenance_routines.py::test_routines_callable_from_non_public_schemafailsintermittently with
asyncpg.exceptions.DeadlockDetectedError. Not new and not tied to anyone branch — most recently on main's own run
32142821174 (2026-08-18),
and again today on an unrelated PR.
Why it kept happening
The test installs the routines into a non-public schema and then calls one — but it loaded
the bodies from
b6d2f8a4c1e7, which has been superseded. #2638's install gating is what thetest is about, and that has been carried forward unchanged. The bodies have not:
c8b4e2a71f95("maintenance routines skip locked schemas") gave each per-schema query alock_timeoutandlock_not_available/deadlock_detectedskip arms, for this exactdeadlock — its own commit message cites this test. The older body has no such guard: it waits
indefinitely for
AccessShareLockon every schema it scans while already holdingAccessShareLockon the ones it scanned earlier. Under xdist, where peer workers create and dropschemas continuously, a peer's
DROPcloses the cycle and PostgreSQL kills one side.So the fix landed, and never reached the test that reports the bug.
Measured directly — victim schema held under
ACCESS EXCLUSIVEfrom a second connection,routine called from a non-public copy with an 8s ceiling:
The fix
walk_revisions(), so the DAG's merge revisions (tupledown_revision) resolve correctly —instead of hard-coding a path that silently rots the next time the bodies are replaced.
test_current_routine_bodies_keep_the_concurrent_ddl_guard: whichever migration owns thebodies must still carry the skip arms. Fast, no DB.
_load_schema_local_migrationstays pinned tob6d2f8a4c1e7for the two tests that assertwhat that migration's install gating emits — those don't execute anything.
test_banks_needing_consolidation_skips_schema_locked_by_ddlalready proves the installedpubliccopy behaves correctly; this just makes the non-public end-to-end test use the same body.Verification
tests/test_maintenance_routines.py: 24 passed.ruff check/ruff format --check/ty check hindsight_api/clean. Tests-only change.