Repository navigation
fix: Transaction rolls back a dead transaction before joining it - #75
Merged
Merged
Conversation
This was referenced Oct 17, 2026
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.
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-enteredTransaction, joined that transaction, and failed withPendingRollbackError, so the second attempt never ran.A transaction counts as dead in two cases:
invalidated. This is what an execute on a lost connection leaves behind. TheSessionTransactionstaysACTIVEandsession.is_activestays 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:session.add()),session.connection()checks one out with default options. A followingTransaction(isolation_level="SERIALIZABLE")was then ignored with a warning, and the block ran at READ COMMITTED.session.connection()raisesUnboundExecutionError, because no statement picks the bind.So the check reads
SessionTransaction._connectionsthroughgetattr. 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:
__aenter__. Transaction can't recover a session whose connection was lost outside the block #74 proposed relaxing the rule to cover queries "before or after" the block, and that turned out to be wrong.Tests
tests/test_transaction.py: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_connectionfixture inconftest.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
TransactionAPI 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 tosession.connection().Performance
_is_deadcosts 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. ATransactionblock withSELECT 1and 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).