Skip to content

fix: Transaction rolls back a dead transaction before joining it - #75

Merged
lesnik512 merged 2 commits into
mainfrom
fix/transaction-recovers-dead-session
Oct 17, 2026
Merged

lesnik512 merged 2 commits into
mainfrom
fix/transaction-recovers-dead-session

Conversation

@lesnik512

@lesnik512 lesnik512 commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

Closes #74.

Transaction.__aenter__ now rolls back a transaction that can no longer continue before it joins it. Before, a connection lost on a query or flush after the block left the session holding a dead transaction. The retry re-entered Transaction, joined that transaction, and failed with PendingRollbackError, so the second attempt never ran.

def _is_dead(session: sa_async.AsyncSession) -> bool:
    transaction = session.sync_session.get_transaction()
    if transaction is None:
        return False
    if not transaction.is_active:
        return True
    held = getattr(transaction, "_connections", {})
    return any(connection.invalidated for connection, *_ in held.values())

A transaction counts as dead in two cases:

  • It is inactive. SQLAlchemy marks the transaction this way after a failed flush, once it has already rolled it back.
  • It holds a connection that is invalidated. This is what an execute on a lost connection leaves behind. The SessionTransaction stays ACTIVE and session.is_active stays True, so only the connection flag shows it.

A live transaction is still joined, as before. That keeps the reason #68 rejected rolling back any open transaction: it would drop a caller's uncommitted work.

Why a private attribute

SQLAlchemy has no public way to see the connections a transaction already holds. The first version of this PR used (await session.connection()).invalidated, and review found two regressions:

  • When the session is in a transaction but has no connection yet (after session.add()), session.connection() checks one out with default options. A following Transaction(isolation_level="SERIALIZABLE") was then ignored with a warning, and the block ran at READ COMMITTED.
  • On a session with only mapper binds, session.connection() raises UnboundExecutionError, because no statement picks the bind.

So the check reads SessionTransaction._connections through getattr. If SQLAlchemy renames it, entering falls back to joining, which is today's behaviour, and the lost-connection tests fail on that release. I asked upstream for a public accessor in sqlalchemy/sqlalchemy#13684.

Limits

The README keeps its rule that the retried coroutine must own its whole transaction. Two cases are still not recoverable:

Tests

tests/test_transaction.py:

  • a transaction whose connection was lost is rolled back on entry;
  • the same after a failed flush;
  • a recovered serializable transaction runs at SERIALIZABLE;
  • entering after autobegin keeps the isolation level;
  • a session with only mapper binds can be entered;
  • a live outer transaction is joined, so its uncommitted row is visible inside the block.

tests/test_retry.py: a connection lost on a query after the block is retried, and the second attempt succeeds.

The idle-connection kill moved into a kill_idle_connection fixture in conftest.py, and the #68 test uses it too.

The lost-connection tests and the three tests the review prompted fail without the change. The live-join test is a guard and passes either way. All 58 tests pass with 100% coverage on SQLAlchemy 2.1.4, and also on 2.0.55 and on 2.0.31, the floor for Python 3.13+. Lint and ty pass.

Docs

README: the retry section names the case that is now recovered, and the Transaction API line describes what entering does. There's no ADR: the change is ten lines in one private function and easy to revert, so it fails the "hard to reverse" test. The rejected alternatives are guarded by tests instead. The live-join test catches rolling back any open transaction, and the autobegin-isolation and mapper-binds tests catch a return to session.connection().

Performance

_is_dead costs 0.06 µs when the session has no transaction, which is the usual case because __aexit__ closes the session, and 0.57 µs when it joins a transaction holding a connection. A Transaction block with SELECT 1 and a commit takes about 150 µs in Docker on one machine. Main and this branch differ by less than the spread between rounds (131 to 208 µs).

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.

Transaction can't recover a session whose connection was lost outside the block

1 participant