summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorMike Bayer <mike_mp@zzzcomputing.com>2014-01-12 19:43:13 -0500
committerMike Bayer <mike_mp@zzzcomputing.com>2014-01-12 19:43:13 -0500
commit9c64607572a04eb2ed7981db8999732100f39d4d (patch)
tree111f47a4d93fcd1c86800d16b9797b2f991d7caa /lib
parentc91fd822bc9816827d0aab4699e304ab49ed8280 (diff)
downloadsqlalchemy-9c64607572a04eb2ed7981db8999732100f39d4d.tar.gz
- :class:`.Connection` now associates a new
:class:`.RootTransaction` or :class:`.TwoPhaseTransaction` with its immediate :class:`._ConnectionFairy` as a "reset handler" for the span of that transaction, which takes over the task of calling commit() or rollback() for the "reset on return" behavior of :class:`.Pool` if the transaction was not otherwise completed. This resolves the issue that a picky transaction like that of MySQL two-phase will be properly closed out when the connection is closed without an explicit rollback or commit (e.g. no longer raises "XAER_RMFAIL" in this case - note this only shows up in logging as the exception is not propagated within pool reset). This issue would arise e.g. when using an orm :class:`.Session` with ``twophase`` set, and then :meth:`.Session.close` is called without an explicit rollback or commit. The change also has the effect that you will now see an explicit "ROLLBACK" in the logs when using a :class:`.Session` object in non-autocommit mode regardless of how that session was discarded. Thanks to Jeff Dairiki and Laurence Rowe for isolating the issue here. [ticket:2907]
Diffstat (limited to 'lib')
-rw-r--r--lib/sqlalchemy/engine/base.py16
-rw-r--r--lib/sqlalchemy/pool.py55
2 files changed, 51 insertions, 20 deletions
diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index ff2e6e282..5c66f4806 100644
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -404,7 +404,7 @@ class Connection(Connectable):
"""
if self.__transaction is None:
- self.__transaction = RootTransaction(self)
+ self.__transaction = self.connection._reset_agent = RootTransaction(self)
return self.__transaction
else:
return Transaction(self, self.__transaction)
@@ -425,7 +425,7 @@ class Connection(Connectable):
"""
if self.__transaction is None:
- self.__transaction = RootTransaction(self)
+ self.__transaction = self.connection._reset_agent = RootTransaction(self)
else:
self.__transaction = NestedTransaction(self, self.__transaction)
return self.__transaction
@@ -453,7 +453,7 @@ class Connection(Connectable):
"is already in progress.")
if xid is None:
xid = self.engine.dialect.create_xid()
- self.__transaction = TwoPhaseTransaction(self, xid)
+ self.__transaction = self.connection._reset_agent = TwoPhaseTransaction(self, xid)
return self.__transaction
def recover_twophase(self):
@@ -491,11 +491,11 @@ class Connection(Connectable):
self.engine.logger.info("ROLLBACK")
try:
self.engine.dialect.do_rollback(self.connection)
- self.__transaction = None
+ self.__transaction = self.connection._reset_agent = None
except Exception as e:
self._handle_dbapi_exception(e, None, None, None, None)
else:
- self.__transaction = None
+ self.__transaction = self.connection._reset_agent = None
def _commit_impl(self, autocommit=False):
if self._has_events:
@@ -505,7 +505,7 @@ class Connection(Connectable):
self.engine.logger.info("COMMIT")
try:
self.engine.dialect.do_commit(self.connection)
- self.__transaction = None
+ self.__transaction = self.connection._reset_agent = None
except Exception as e:
self._handle_dbapi_exception(e, None, None, None, None)
@@ -560,7 +560,7 @@ class Connection(Connectable):
if self._still_open_and_connection_is_valid:
assert isinstance(self.__transaction, TwoPhaseTransaction)
self.engine.dialect.do_rollback_twophase(self, xid, is_prepared)
- self.__transaction = None
+ self.__transaction = self.connection._reset_agent = None
def _commit_twophase_impl(self, xid, is_prepared):
if self._has_events:
@@ -569,7 +569,7 @@ class Connection(Connectable):
if self._still_open_and_connection_is_valid:
assert isinstance(self.__transaction, TwoPhaseTransaction)
self.engine.dialect.do_commit_twophase(self, xid, is_prepared)
- self.__transaction = None
+ self.__transaction = self.connection._reset_agent = None
def _autorollback(self):
if not self.in_transaction():
diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py
index 0f0a2ac10..f84f331d5 100644
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -479,18 +479,8 @@ def _finalize_fairy(connection, connection_record, pool, ref, echo, fairy=None):
try:
fairy = fairy or _ConnectionFairy(connection, connection_record)
- if pool.dispatch.reset:
- pool.dispatch.reset(fairy, connection_record)
- if pool._reset_on_return is reset_rollback:
- if echo:
- pool.logger.debug("Connection %s rollback-on-return",
- connection)
- pool._dialect.do_rollback(fairy)
- elif pool._reset_on_return is reset_commit:
- if echo:
- pool.logger.debug("Connection %s commit-on-return",
- connection)
- pool._dialect.do_commit(fairy)
+ assert fairy.connection is connection
+ fairy._reset(pool, echo)
# Immediately close detached instances
if not connection_record:
@@ -542,6 +532,23 @@ class _ConnectionFairy(object):
"""
+ _reset_agent = None
+ """Refer to an object with a ``.commit()`` and ``.rollback()`` method;
+ if non-None, the "reset-on-return" feature will call upon this object
+ rather than directly against the dialect-level do_rollback() and do_commit()
+ methods.
+
+ In practice, a :class:`.Connection` assigns a :class:`.Transaction` object
+ to this variable when one is in scope so that the :class:`.Transaction`
+ takes the job of committing or rolling back on return if
+ :meth:`.Connection.close` is called while the :class:`.Transaction`
+ still exists.
+
+ This is essentially an "event handler" of sorts but is simplified as an
+ instance variable both for performance/simplicity as well as that there
+ can only be one "reset agent" at a time.
+ """
+
@classmethod
def _checkout(cls, pool, threadconns=None, fairy=None):
if not fairy:
@@ -591,6 +598,30 @@ class _ConnectionFairy(object):
_close = _checkin
+ def _reset(self, pool, echo):
+ if pool.dispatch.reset:
+ pool.dispatch.reset(self, self._connection_record)
+ if pool._reset_on_return is reset_rollback:
+ if echo:
+ pool.logger.debug("Connection %s rollback-on-return%s",
+ self.connection,
+ ", via agent"
+ if self._reset_agent else "")
+ if self._reset_agent:
+ self._reset_agent.rollback()
+ else:
+ pool._dialect.do_rollback(self)
+ elif pool._reset_on_return is reset_commit:
+ if echo:
+ pool.logger.debug("Connection %s commit-on-return%s",
+ self.connection,
+ ", via agent"
+ if self._reset_agent else "")
+ if self._reset_agent:
+ self._reset_agent.commit()
+ else:
+ pool._dialect.do_commit(self)
+
@property
def _logger(self):
return self._pool.logger