feat(unified): pin the backend at the first statement, not at atomic() entry
Summary
A competing take on !25 (closed)'s pinning strategy. Everything about unified sessions stays as it is; the one thing that changes is when a transaction takes a connection out of the pool.
!25 (closed) pins at atomic() entry. That was load-bearing for two separate jobs, and
only one of them actually needs a connection:
- Ownership — the block that opens the transaction is the block that ends it.
- The fork/join oracle —
session.tx is not Nonewas how a child task learned it was running inside a transaction.
Job 2 is a question, not a resource. Entry now stamps the session's transaction
epoch into a contextvar and pins nothing; the first statement that needs a
backend pins one, where Django on psycopg issues its BEGIN. Job 1 still holds
because __exit__ is what commits, and a block that never queried has nothing
to commit.
Why bother
Eager pinning taxes the interval between atomic() entry and the block's first
SQL statement. That is zero for a block that queries immediately — and the whole
of a template render under ATOMIC_REQUESTS, or a payment API call made inside
the block. By Little's Law that interval is connections.
benchmarks/synthetic/pin_hold.py (added here) measures both arms from one
checkout: BENCH_EAGER=1 force-pins at entry, reproducing what !25 (closed) does.
120 concurrent tasks against a pool of 8, medians of 21 runs:
| non-DB work in the block | async eager / lazy | sync eager / lazy | control eager / lazy |
|---|---|---|---|
| 10 ms | 0.316 / 0.124 | 0.231 / 0.114 | 0.132 / 0.128 |
| 50 ms | 0.931 / 0.159 | 0.889 / 0.155 | 0.134 / 0.124 |
| 200 ms | 3.307 / 0.327 | 3.275 / 0.321 | 0.134 / 0.126 |
Read the ratio as a shape, not a headline. The eager floor is
ceil(tasks / pool) × hold and the lazy floor is hold, so the ratio is
tasks / pool in the limit by construction and grows without bound as you
turn either knob. What the benchmark shows is that hold time moves from
entry→exit to first-statement→exit; how much that is worth is a property of the
workload, not of the driver.
The control is the column that matters. A block that queries immediately ties at every hold time — there is no hold to remove, so there is no workload where this change costs throughput.
Three secondary effects go away with it: idle in transaction and the vacuum
horizon stop running for blocks that have not touched the database yet, and a
pool timeout raises from the first statement again rather than from
atomic()'s __enter__, where a try/except around the block body cannot
see it.
What changed
mark_transaction_open()is idempotent per transaction and fires from the autocommit setters as well as thetxsetter. The second caller must not draw a fresh epoch — that invalidates the marks children forked after entry are already carrying and drops every one of them out of the transaction they are running in.clear_transaction_mark()resets the epoch so a block that never queried is still unmarked.may_use()testswants_transaction()rather thantx is not None. A child forked into a block that has not queried yet is inside it.- The async statement path gates on the resolved shared session (
_wants_tx) instead of on its own connection'sautocommitflag, which is per-connection and blind to a transaction the sync world opened. - The first
BEGINis serialised onSharedSession.begin_lockrather than on either connection's own lock — see "Found in review" below. - Entering a transaction releases this Connection's autocommit session pin.
The third piece is not optional. Without it, removing the eager pin reopens two
holes immediately: a sync atomic() that has not queried stops capturing the
first async write, and a child's async_atomic() emits SAVEPOINT with no
transaction open (25P01). Both surfaced as test failures on the first run.
Found in review — two critical bugs, both fixed here
Fresh-context adversarial review after the branch was first pushed. Neither bug was caught by the 414 tests that were green at the time.
Two units of work could both open the transaction — silent data loss
tx lives on the SharedSession, but the lock guarding the check-open-assign
that fills it lived on the connection. Units that fork before anything
materialises async_connections each build their own wrapper, and so their own
lock. Both read tx is None, both BEGIN, and the second assignment overwrites
the first — taking everything already dispatched on the loser with it.
with transaction.atomic():
async_to_sync(lambda: asyncio.gather(a(), b()))() # committed 1 row of 2Confirmed by A/B: feat/unified-ownership keeps both rows, this branch kept
one. The eager pin filled tx before any child could run, so this branch is
what exposed it — a lost write, not a leak.
Fixed by moving the lock beside the thing it guards. SharedSession.begin_lock
is a threading.Lock, because the contenders span the sync world, the loop
thread and sync_to_async workers. Async callers take it inside the executor
the blocking BEGIN already runs in, then stamp the transaction mark back on
the calling context — the worker thread has none of its contextvars, and a
joiner that lost the race never assigns at all.
One Django connection held two pool slots
_begin_tx gives back the autocommit session pin when that Connection opens
the transaction. When the async wrapper opened it instead, the Connection sat on
a second slot for the rest of the block; a middleware query followed by an async
view is enough. On a tight pool that is a hang, not a capacity regression — and
it made this MR's headline claim false on the entire sync path, including the
ATOMIC_REQUESTS example. Now:
after autocommit SELECT : pin held = True
inside atomic(), no statement: pin held = False, tx = False
after first statement : pin held = False, tx = True
after the block : pin held = False, tx = FalseAUTOCOMMIT=False never stamped the mark
connect() runs detached, so the flag was already False by the time the
wrapper set it again and the setter short-circuited. The session then claimed a
transaction with epoch 0, and every task forked before the first statement
forked its own session out of a connection permanently in one.
Also fixed: the escape hatch in the docs has never worked
Pre-existing on both MRs. The docs told users to reach for
async_connections._independent_connection() when a child genuinely wants its
own transaction. It swaps a fresh wrapper in under the same alias, this registry
is keyed by alias, so the child finds the same session and joins the transaction
it was trying to escape — its writes then roll back with the parent, silently.
Its replacement async_new_connection(), released in 6.1.2, behaves the same
way — and since that is now also what upstream recommends for parallel queries,
the note matters more than it did. A second DATABASES alias does work; the docs, the savepoint error message
and a test now say so.
django-async-backend 6.1.2 — released, and this branch is specified against it
What was PR #78 shipped as 6.1.2 while this MR was open. It stamps every async connection with the task that created it and checks the stamp on every statement (_prepare_cursor, commit, rollback, savepoint), removes _independent_connection(), and adds async_new_connection() as the sanctioned way to get a connection of your own. The async extra floor moves to 6.1.2 here, because that is the ownership contract these tests specify.
The branch assumes that floor rather than negotiating with it: no fallbacks for older releases, .gitlab-ci.yml states the same floor as pyproject.toml so a resolver cannot quietly test an older one, and the tests and docs describe the ownership contract in the present tense rather than as a release delta. Version history stays in the changelog.
Re-run against the real release rather than the shadowed PR: 411 passed (py3.14, live PG 17). The three failures the shadow predicted were the three tests that asserted the old contract; they are re-aimed, not deleted:
| test | old contract | now |
|---|---|---|
..._forked_before_the_first_statement_still_joins |
a child forked in the lazy window joins the transaction | renamed ..._forked_inside_the_block_is_refused_cleanly — upstream refuses it; what is pinned is that the refusal cannot leave the block able to commit part of its work |
test_gather_inside_a_transaction_joins_it |
upstream documented the shape as supported | split into ..._bare_gather_..._is_refused and test_async_new_connection_children_join_the_parents_transaction |
test_upstreams_nested_task_guard_still_fires |
matched the old error wording | matches the new one; the guard fires the same |
The joining direction is still live, so SharedSession.begin_lock is not dead code. The guard compares the wrapper's owner, so it does not fire when the child built its own wrapper — which is exactly what happens when the transaction was opened from the sync world and nothing had materialised async_connections at fork time. test_two_children_racing_the_first_statement_both_land_in_the_tx still passes on 6.1.2, and still fails without the session-scoped lock. That was the critical bug this MR exposed, and 6.1.2 does not retire it.
Two behaviours 6.1.2 introduces, both measured here and newly pinned:
async_new_connection()does not part the session. It satisfies the ownership stamp — the wrapper is created inside the child — but the session registry is keyed by alias, so upstream's sanctioned parallel fan-out lands every child on the caller's session, inside the caller's transaction, taking turns on its one backend. Measured: three children, one transaction, all three rows visible to the parent and all three rolled back with it. Correct, and safer than one unowned transaction per child, but it is not the parallelism upstream's docs promise; a secondDATABASESalias is what buys that. Documented indocs/unified-sessions.md.- A straggler that outlives the block is refused rather than committed. The
gather-doesn't-cancel-siblings hole (#77) closes for any child that inherited the wrapper: its post-block write used to land on the autocommit pin and survive the rollback, and now raises. It stays open for a child that built its own wrapper, so that caveat is narrowed rather than deleted.
One thing to know either way: catching the RuntimeError yourself does not save the block. The refused write went through Django's mark_for_rollback_on_error, so a single stray gather inside a transaction costs the whole block. That is the safe outcome, and it is now asserted.
It also settles the argument with !25 (closed): eager pinning's one remaining structural advantage was closing the first-BEGIN race in gather(sql, sql), and that shape is now refused upstream — while the race that does survive is the sync-world one, which eager pinning never closed either.
Testing
- django-vpg: see the Testing section below for current counts.
- Three new regression tests, each confirmed to fail without its fix. The sync/async race needs 100 repetitions to be reliable — at 25 it only caught the bug two runs in three, which is worse than no test.
- glitchtip-backend, full suite against this branch: 1534 tests, no hang,
FAILED (failures=4, errors=38, skipped=2). The 38 errors are the Rust ingest tests failing on Valkey DNS; all four failures reproduce identically when the same run is pointed atfeat/unified-ownership, so nothing here fails that eager pinning does not also fail. This is the run that matters for the new cross-world lock — the historical failure mode for this feature is a suite that hangs rather than one that fails. - Three of !25 (closed)'s own tests asserted eager pinning and were rewritten, not
deleted: pin-at-entry became pin-at-first-statement,
TestEagerPinFailurebecameTestFailedPin(the failure moved to the statement path), and the kill-switch test lost its premise so it now discriminates on the shared session itself.
Risks
This re-enables the predicate that 14075814 removed, and that commit's
message records it deadlocking glitchtip's suite: a bare async
INSERT ... ON CONFLICT under a TestCase's outer atomic opened a transaction
nobody owned, and the next TransactionTestCase's TRUNCATE blocked on it
forever. The reason it is safe now is that 14075814 predates the ownership
rework — a task forked outside the block resolves to a fresh SharedSession
whose defaults make wants_transaction False, so it opens nothing, while a
joining task resolves to the session the block owns. Both reviewers probed this
independently and agreed the predicate itself is sound; the glitchtip run is the
empirical half.
A lock now spans the sync and async worlds. SharedSession.begin_lock is
held across a blocking driver.begin(). Async callers take it inside an
executor so the event loop never blocks on it, but a sync caller waiting for a
pool slot does hold it, and an async caller then occupies an executor thread
waiting. That is bounded by pool contention rather than unbounded, and the
glitchtip run exercises it, but it is the part of this change most worth a
second pair of eyes.
Round two: no global switch, and a second review pass
VPG_UNIFIED_SESSIONS is gone (breaking). Unified sessions are how the
driver works. The switch offered a mode that is a data-integrity bug — two
connections per alias, where a TestCase's rollback does not cover the async
side — and calling that "restores the previous behaviour" invited people to
trade a hang for silent divergence. OPTIONS={"pool": False} remains the
per-alias opt-out, at the cost of pooling for that alias.
Deleting it removed ~40 lines and no downstream branch: session is None is
still reached through pool: False, a bare django_vpg.dbapi user, and
unified.detached(). The control test that demonstrated the feature is
load-bearing did not need the flag either — it now runs against a pool: False alias as TestPoolFalseRestoresTheOldBreakage. Only the flag-parsing
unit tests are actually gone.
A second adversarial pass then found three more code bugs, all fixed:
- A cancelled task stranded a live
BEGIN.await run_in_executor(...)is a cancellation point and the worker thread is not cancellable, so cancelling mid-BEGINlet the block unwind first, roll back nothing, and the worker then publish its transaction to a session nobody owned.asyncio.timeout,wait_forand ASGI client-disconnect all reach it. Shielded and drained now, so__aexit__finds it. BEGINwas running on the loop's default executor, where a slow one also stallsloop.getaddrinfo,asyncio.to_threadand every otherrun_in_executor(None, ...). It has its own pool now.- The race loser kept its autocommit pin — and with a session-wide lock, losing is the normal outcome for every contender after the first.
Plus: AUTOCOMMIT=False had no test (reverting its fix left the suite green
— now covered), and the sync/async race test was a 50% coin flip when the
module ran in order. My "100 reps, 5/5" measurement for it was taken with
-k, in isolation, which is not how CI runs it. Replaced with a
deterministic assertion of the invariant.
Four documentation claims were wrong
All measured, all corrected in 36f2278:
- "Concurrent threads are detected, not silently allowed" — false in the
worst case. Two concurrent
thread_sensitive=Falseworkers that each open their own outermostatomic()merge into one transaction; the second's block exits successfully and its rows are then discarded by the first's rollback. Nothing raises. 5/5 deterministic, and the same commit sticks on an opted-out alias, so this feature causes it. It is documented as a limitation now and pinned by a test. - "A pool timeout or a connection error still raises from the first
statement" — connection errors still raise from
__enter__, on this branch and the parent alike, becauseset_autocommit()callsensure_connection(). - "Each world keeps its own autocommit pin" — only the sync world has one. The async side takes a pooled backend per statement, so TEMP tables and GUCs do not survive between two consecutive async statements.
- The detection section described a mechanism the code explicitly rejects and quoted an error string that does not exist in the tree.
The benchmark was independently re-verified as faithful, including the
decisive control: with BENCH_POOL above BENCH_TASKS the two arms tie, and
with a tight pool eager is ~6x slower. So the gap is pool-slot hold time and
nothing else. The docstring no longer claims the ratio is TASKS / POOL "by
construction" — measured is about half that.
Known and not fixed
One Django connection can still hold two pool slots when the async side
opens the transaction. The fix went in for the sync direction only:
atomic() entry and _begin_tx hand the pin back, but nothing releases a
sync Connection's pin when async_atomic() is what opens the transaction.
Middleware query then an async write needs two slots, and hangs on a pool of
one.
The fix needs the session to know its pin holders and release them from whichever thread opens the transaction. That is safe in every supported configuration — the holder is always quiescent, because concurrent units are exactly the unsupported case above — but it is a cross-thread release of a pooled resource next to a lock this MR has already had to fix twice, so it wants its own review rather than being tacked on at the end of this one. Filed rather than rushed; the MR is Draft anyway.
Testing
411 tests pass (py3.14, live PG 17, django-async-backend 6.1.2);
scripts/check.sh clean. Every regression test added in this MR is confirmed
to fail without its fix.
glitchtip-backend is not exposed to the 6.1.2 change either way — it pins
django-async-backend>=6.0,<6.1, so the ownership guard arrives there with the
Django 6.1 upgrade, not with this.
Relationship to !25 (closed)
This branch is feat/unified-ownership plus the commits listed in the compare
view. It is targeted at main so the two are genuinely alternative merge
candidates.
Note that origin/feat/unified-ownership is currently one commit behind local
(98fa893), so the comparison view will be slightly off until that is pushed.
AI disclosure: Claude Opus 5 wrote the implementation, the tests, the benchmark and this description, ran the measurements, the adversarial reviews, and the 6.1.2 compatibility work.