- changed status to resolved
clean out XID state on connection.close()
On Friday, 10 January 2014 08:06:16 UTC-8, Michael Bayer wrote: maybe. I was thinking, is the Session really doing the right thing here by not getting involved, but I think yeah that still might be appropriate. so we’d need to carry along some extra information about the transaction with the connection so that do_rollback() and do_commit() can pick up on that.
or maybe session.close() needs to do some bookkeeping with the SessionTransaction. not sure.
The call stack here is:
-> session.close() sqlalchemy/orm/session.py(999)close() -> transaction.close() sqlalchemy/orm/session.py(437)close() -> connection.close() sqlalchemy/engine/base.py(583)close() -> conn.close() sqlalchemy/pool.py(579)close() -> self.checkin() sqlalchemy/pool.py(506)checkin() -> self._pool, None, self._echo, fairy=self) sqlalchemy/pool.py(432)_finalize_fairy() -> pool._dialect.do_rollback(fairy)
The tpc state (xid, is_prepared) is held on the TwoPhaseTransaction object, referenced (self.__transaction) by the engine Connection object which calls conn.close(). I don't think SessionTransaction need be involved here, but perhaps the engine Connection / Transaction should. Some options are:
-
In engine Connection.close(), when self.__transaction.is_active pass down xid, is_prepared to the _ConnectionFairy. That would imply muddying the DBAPI Connection contract of the engine Connection.__connection.
-
Make the engine Transaction aware of the configured reset_on_return behaviour so that Transaction.close() can either rollback or commit. Connection.close() could then call Transaction.close().
-
Keep track of xid, is_prepared on the pool's connection_record.
Comments (4)
-
reporter -
reporter more adjustments and fixes in bebf30e34d669a5ede54e512e55.
-
reporter - Fixed some "double invalidate" situations were detected where
a connection invalidation could occur within an already critical section
like a connection.close(); ultimately, these conditions are caused
by the change in
2907
, in that the "reset on return" feature calls out to the Connection/Transaction in order to handle it, where "disconnect detection" might be caught. However, it's possible that the more recent change in2985
made it more likely for this to be seen as the "connection invalidate" operation is much quicker, as the issue is more reproducible on 0.9.4 than 0.9.3.
Checks are now added within any section that an invalidate might occur to halt further disallowed operations on the invalidated connection. This includes two fixes both at the engine level and at the pool level. While the issue was observed with highly concurrent gevent cases, it could in theory occur in any kind of scenario where a disconnect occurs within the connection close operation. fixes
#3043ref#2985ref#2907- add some defensive checks during an invalidate situation: 1. _ConnectionRecord.invalidate might be called twice within finalize_fairy if the _reset() raises an invalidate condition, invalidates, raises and then goes to invalidate the CR. so check for this. 2. similarly within Conneciton, anytime we do handle_dbapi_error(), we might become invalidated. so a following finally must check self.__invalid before dealing with the connection any futher.
→ <<cset 85d1899b76a3>>
- Fixed some "double invalidate" situations were detected where
a connection invalidation could occur within an already critical section
like a connection.close(); ultimately, these conditions are caused
by the change in
-
reporter - changed milestone to 1.0.xx
- Log in to comment
9c64607572a04eb2ed7981db8999