diff options
| -rw-r--r-- | doc/build/changelog/changelog_11.rst | 15 | ||||
| -rw-r--r-- | doc/build/changelog/migration_11.rst | 40 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/session.py | 43 | ||||
| -rw-r--r-- | test/orm/test_transaction.py | 33 |
4 files changed, 116 insertions, 15 deletions
diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index 178383b85..0a5dc3ea0 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -22,6 +22,21 @@ :version: 1.1.0b1 .. change:: + :tags: bug, orm, mysql + :tickets: 3680 + + Further continuing on the common MySQL exception case of + a savepoint being cancelled first covered in :ticket:`2696`, + the failure mode in which the :class:`.Session` is placed when a + SAVEPOINT vanishes before rollback has been improved to allow the + :class:`.Session` to still function outside of that savepoint. + It is assumed that the savepoint operation failed and was cancelled. + + .. seealso:: + + :ref:`change_3680` + + .. change:: :tags: feature, mssql :tickets: 3534 diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst index 3e839af63..cca2b1ae8 100644 --- a/doc/build/changelog/migration_11.rst +++ b/doc/build/changelog/migration_11.rst @@ -16,7 +16,7 @@ What's New in SQLAlchemy 1.1? some issues may be moved to later milestones in order to allow for a timely release. - Document last updated: Feburary 25, 2016 + Document last updated: March 23, 2016 Introduction ============ @@ -290,6 +290,44 @@ time on the outside of the subquery. :ticket:`3582` +.. _change_3680: + +Improved Session state when a SAVEPOINT is cancelled by the database +-------------------------------------------------------------------- + +A common case with MySQL is that a SAVEPOINT is cancelled when a deadlock +occurs within the transaction. The :class:`.Session` has been modfied +to deal with this failure mode slightly more gracefully, such that the +outer, non-savepoint transaction still remains usable:: + + s = Session() + s.begin_nested() + + s.add(SomeObject()) + + try: + # assume the flush fails, flush goes to rollback to the + # savepoint and that also fails + s.flush() + except Exception as err: + print("Something broke, and our SAVEPOINT vanished too") + + # this is the SAVEPOINT transaction, marked as + # DEACTIVE so the rollback() call succeeds + s.rollback() + + # this is the outermost transaction, remains ACTIVE + # so rollback() or commit() can succeed + s.rollback() + +This issue is a continuation of :ticket:`2696` where we emit a warning +so that the original error can be seen when running on Python 2, even though +the SAVEPOINT exception takes precedence. On Python 3, exceptions are chained +so both failures are reported individually. + + +:ticket:`3680` + .. _change_3677: Erroneous "new instance X conflicts with persistent instance Y" flush errors fixed diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 48ff09b87..dc5de7ac6 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -235,7 +235,7 @@ class SessionTransaction(object): return SessionTransaction( self.session, self, nested=nested) - def _iterate_parents(self, upto=None): + def _iterate_self_and_parents(self, upto=None): current = self result = () @@ -269,6 +269,11 @@ class SessionTransaction(object): self._key_switches = weakref.WeakKeyDictionary() def _restore_snapshot(self, dirty_only=False): + """Restore the restoration state taken before a transaction began. + + Corresponds to a rollback. + + """ assert self._is_transaction_boundary self.session._expunge_states( @@ -290,6 +295,11 @@ class SessionTransaction(object): s._expire(s.dict, self.session.identity_map._modified) def _remove_snapshot(self): + """Remove the restoration state taken before a transaction began. + + Corresponds to a commit. + + """ assert self._is_transaction_boundary if not self.nested and self.session.expire_on_commit: @@ -358,7 +368,7 @@ class SessionTransaction(object): stx = self.session.transaction if stx is not self: - for subtransaction in stx._iterate_parents(upto=self): + for subtransaction in stx._iterate_self_and_parents(upto=self): subtransaction.commit() if not self.session._flushing: @@ -405,14 +415,18 @@ class SessionTransaction(object): stx = self.session.transaction if stx is not self: - for subtransaction in stx._iterate_parents(upto=self): + for subtransaction in stx._iterate_self_and_parents(upto=self): subtransaction.close() boundary = self + rollback_err = None if self._state in (ACTIVE, PREPARED): - for transaction in self._iterate_parents(): + for transaction in self._iterate_self_and_parents(): if transaction._parent is None or transaction.nested: - transaction._rollback_impl() + try: + transaction._rollback_impl() + except: + rollback_err = sys.exc_info() transaction._state = DEACTIVE boundary = transaction break @@ -421,7 +435,7 @@ class SessionTransaction(object): sess = self.session - if sess._enable_transaction_accounting and \ + if not rollback_err and sess._enable_transaction_accounting and \ not sess._is_clean(): # if items were added, deleted, or mutated @@ -433,19 +447,24 @@ class SessionTransaction(object): boundary._restore_snapshot(dirty_only=boundary.nested) self.close() + if self._parent and _capture_exception: self._parent._rollback_exception = sys.exc_info()[1] + if rollback_err: + util.reraise(*rollback_err) + sess.dispatch.after_soft_rollback(sess, self) return self._parent def _rollback_impl(self): - for t in set(self._connections.values()): - t[1].rollback() - - if self.session._enable_transaction_accounting: - self._restore_snapshot(dirty_only=self.nested) + try: + for t in set(self._connections.values()): + t[1].rollback() + finally: + if self.session._enable_transaction_accounting: + self._restore_snapshot(dirty_only=self.nested) self.session.dispatch.after_rollback(self.session) @@ -1078,7 +1097,7 @@ class Session(_SessionClassMethods): def _close_impl(self, invalidate): self.expunge_all() if self.transaction is not None: - for transaction in self.transaction._iterate_parents(): + for transaction in self.transaction._iterate_self_and_parents(): transaction.close(invalidate) def expunge_all(self): diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py index c1662c9d1..d912c669f 100644 --- a/test/orm/test_transaction.py +++ b/test/orm/test_transaction.py @@ -3,10 +3,10 @@ from sqlalchemy import ( testing, exc as sa_exc, event, String, Column, Table, select, func) from sqlalchemy.testing import ( fixtures, engines, eq_, assert_raises, assert_raises_message, - assert_warnings, mock, expect_warnings) + assert_warnings, mock, expect_warnings, is_, is_not_) from sqlalchemy.orm import ( exc as orm_exc, Session, mapper, sessionmaker, create_session, - relationship, attributes) + relationship, attributes, session as _session) from sqlalchemy.testing.util import gc_collect from test.orm._fixtures import FixtureTest from sqlalchemy import inspect @@ -1290,6 +1290,35 @@ class SavepointTest(_LocalFixture): assert u1 in s assert u1 not in s.deleted + @testing.requires.savepoints + def test_savepoint_lost_still_runs(self): + User = self.classes.User + s = self.session(bind=self.bind) + trans = s.begin_nested() + s.connection() + u1 = User(name='ed') + s.add(u1) + + # kill off the transaction + nested_trans = trans._connections[self.bind][1] + nested_trans._do_commit() + + is_(s.transaction, trans) + assert_raises( + sa_exc.DBAPIError, + s.rollback + ) + + assert u1 not in s.new + + is_(trans._state, _session.CLOSED) + is_not_(s.transaction, trans) + is_(s.transaction._state, _session.ACTIVE) + + is_(s.transaction.nested, False) + + is_(s.transaction._parent, None) + class AccountingFlagsTest(_LocalFixture): __backend__ = True |