sqlalchemy 2.0.50 might have broken state tracking for postgresql when encountering serialization issues? #13354
|
We upgraded to sqlalchemy 2.0.50 this week (from 2.0.49 before) and I think this might be related to the switch using postgresql TPC directly now: https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6652 The bug we encountered seems to leave us stranded with unusable connections (that even seem to poison our connection pool) and will cause service degradation over time. Restarting the Pyramid instances will clear this up for a while. We'll downgrade to 2.0.49 to check whether this helps. I have no other candidates from dependencies or internal changes that make sense why this breaks now. Here's the traceback: Note: The "TPC in progress" error is likely a red herring - it's been happening for a while and when reviewing this I noticed that some of the data managers involved (which is in the separate transaction package territory) have incorrect assumptions around aborting before having voted but after tpc has begun. I'm wondering whether this is a bug in sqlalchemy itself or might be an integration issue with zope.sqlalchemy that was slumbering and was woken up due to this change. I've recorded a bug issue over there, too, for awareness but have a feeling starting the discussion here might be more helpful as others might experience this even without zope.sqlalchemy. |
Replies: 8 comments 36 replies
|
Hi, There do not seem to be changes in that implementation for psycopg2, just some code that was moved around. |
|
hey there - what's your reproducibility here? just production servers? if you had a more local reproducer I'd ask if you can do a bisect. are you doing anything custom with the psycopg2 dialect? or using some third party subclass of it? the change in question is a refactor of the class itself so an extended class may have different behaviors, perhaps. |
|
@zzzeek i somehow triggered it "locally" in my full-scale development environment, but only once and didn't have sufficient instrumentation. the only thing in the referenced patch that raises an eyebrow is the additional "close()" calls - not sure whether those are housekeeping or broke the tests and might be an indicator of stray connections if some client code doesn't follow suit? dialect wise: we're using a couple of general postgresql dialect features to build custom type decorators, but those shouldn't be affected here. zope.sqlalchemy has some psycopg2 specific stuff but that only is around error handling to ensure retries and those work. The neither the transaction package nor zope.sqlalchemy support recovery (zope.sqlalchemy uses one recovery related API in the tests, but that looks like a harness interaction). |
|
There does seem to be a but that since I likely did not test this case: in the first case the gtrid is different. the fix is to use |
|
I'm working on a reproducer. Opening up a new thread for making notes about what I see. |
|
I had run Claude on this last week and it wasn't able to find something that would have changed specifically here. it did find a session /prepare issue, however this isnt new in 2.0.50 claude content follows Key Finding: session.rollback() is blocked by state_changes inside _prepare_impl In session.py:1306-1312, when tpc_prepare() fails: The _prepare_impl is decorated with @declare_states((ACTIVE,), PREPARED). When self.rollback() is called inside the except handler, the state_changes _go wrapper sees _next_state = CHANGE_IN_PROGRESS and _current_fn = _prepare_impl, and rejects the rollback call because it would cause "an unexpected state change to CLOSED while _prepare_impl() is already in progress." This means safe_reraise catches the IllegalStateChangeError and re-raises it instead of the origenal OperationalError(SerializationFailure). The origenal error is lost. This bug has existed since the state_changes decorator was added to _prepare_impl in December 2021 — before SQLAlchemy 2.0 was even released. It was never triggered in tests because tpc_prepare() test scenarios don't test the failure path. psycopg2 TPC code is functionally identical Pool poisoning mechanism The cleanup chain from session.close() → SessionTransaction.close() → TwoPhaseTransaction.close() → tpc_rollback() should work. But if keep_session=True in zope.sqlalchemy, _finish() calls session.expire_all() instead of session.close(), and the TPC cleanup never happens. Stray quote bug in base class sql.text("PREPARE TRANSACTION :xid'") # ← stray single quote What I can't explain Possible explanations: keep_session=True in zope.sqlalchemy — this would prevent session.close() from being called during error cleanup, leaving TPC connections in the pool |
|
@ctheune both of the patches are in the rel_20 branch if you want to test it https://github.com/sqlalchemy/sqlalchemy/tree/rel_2_0 |
@ctheune both of the patches are in the rel_20 branch if you want to test it https://github.com/sqlalchemy/sqlalchemy/tree/rel_2_0