Skip to content

fix(tests): run the maintenance-routine e2e against the current routine body - #3680

Merged
nicoloboschi merged 1 commit into
mainfrom
fix/maintenance-routines-e2e-uses-current-body
Aug 21, 2026
Merged

fix(tests): run the maintenance-routine e2e against the current routine body#3680
nicoloboschi merged 1 commit into
mainfrom
fix/maintenance-routines-e2e-uses-current-body

Conversation

@nicoloboschi

Copy link
Copy Markdown
Collaborator

The flake

test_maintenance_routines.py::test_routines_callable_from_non_public_schema fails
intermittently with asyncpg.exceptions.DeadlockDetectedError. Not new and not tied to any
one branch — most recently on main's own run
32142821174
(2026-08-18),
and again today on an unrelated PR.

Process 4732 waits for AccessShareLock on relation 20240; blocked by process 4645.
Process 4645 waits for AccessExclusiveLock on relation 20256; blocked by process 4732.

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 the
test is about, and that has been carried forward unchanged. The bodies have not:

c8b4e2a71f95 ("maintenance routines skip locked schemas") gave each per-schema query a
lock_timeout and lock_not_available / deadlock_detected skip arms, for this exact
deadlock
— its own commit message cites this test. The older body has no such guard: it waits
indefinitely for AccessShareLock on every schema it scans while already holding
AccessShareLock on the ones it scanned earlier. Under xdist, where peer workers create and drop
schemas continuously, a peer's DROP closes 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 EXCLUSIVE from a second connection,
routine called from a non-public copy with an 8s ceiling:

OLD body (b6d2f8a4c1e7, what the test installed): BLOCKED
NEW body (current chain head definition):         returned

The fix

  • Resolve the migration that owns the current bodies from the revision chain — via Alembic's
    walk_revisions(), so the DAG's merge revisions (tuple down_revision) resolve correctly —
    instead of hard-coding a path that silently rots the next time the bodies are replaced.
  • Add test_current_routine_bodies_keep_the_concurrent_ddl_guard: whichever migration owns the
    bodies must still carry the skip arms. Fast, no DB.
  • _load_schema_local_migration stays pinned to b6d2f8a4c1e7 for the two tests that assert
    what that migration's install gating emits — those don't execute anything.

test_banks_needing_consolidation_skips_schema_locked_by_ddl already proves the installed
public copy 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.

…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.
@nicoloboschi
nicoloboschi merged commit c290d7c into main Aug 21, 2026
108 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant