From 60b5b14988b2986c161e6a25dc5030a31d41728b Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 13 Jun 2016 18:56:17 -0400 Subject: Memoize sys.exc_info() before attempting a savepoint rollback This presents a system of reporting the ultimate "cause" in the case that a MySQL failed SAVEPOINT rollback occurs. Change-Id: Iea834429a0b8398f076b10426525a5c17dbf3c85 --- oslo_db/exception.py | 7 +-- oslo_db/sqlalchemy/exc_filters.py | 46 ++++++++++++++++++- oslo_db/tests/sqlalchemy/test_exc_filters.py | 68 ++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/oslo_db/exception.py b/oslo_db/exception.py index 16dec7f..2d118cd 100644 --- a/oslo_db/exception.py +++ b/oslo_db/exception.py @@ -47,9 +47,10 @@ import debtcollector.removals import six from oslo_db._i18n import _ +from oslo_utils.excutils import CausedByException -class DBError(Exception): +class DBError(CausedByException): """Base exception for all custom database exceptions. @@ -57,9 +58,9 @@ class DBError(Exception): DBError or its subclasses. """ - def __init__(self, inner_exception=None): + def __init__(self, inner_exception=None, cause=None): self.inner_exception = inner_exception - super(DBError, self).__init__(six.text_type(inner_exception)) + super(DBError, self).__init__(six.text_type(inner_exception), cause) class DBDuplicateEntry(DBError): diff --git a/oslo_db/sqlalchemy/exc_filters.py b/oslo_db/sqlalchemy/exc_filters.py index 1aeada7..466a733 100644 --- a/oslo_db/sqlalchemy/exc_filters.py +++ b/oslo_db/sqlalchemy/exc_filters.py @@ -14,6 +14,7 @@ import collections import logging import re +import sys from sqlalchemy import event from sqlalchemy import exc as sqla_exc @@ -383,6 +384,8 @@ def _raise_for_all_others(error, match, engine_name, is_disconnect): LOG.exception(_LE('DB exception wrapped.')) raise exception.DBError(error) +ROLLBACK_CAUSE_KEY = 'oslo.db.sp_rollback_cause' + def handler(context): """Iterate through available filters and invoke those which match. @@ -416,14 +419,53 @@ def handler(context): match, context.engine.dialect.name, context.is_disconnect) - except exception.DBConnectionError: - context.is_disconnect = True + except exception.DBError as dbe: + if ( + context.connection is not None and + not context.connection.closed and + not context.connection.invalidated and + ROLLBACK_CAUSE_KEY + in context.connection.info + ): + dbe.cause = \ + context.connection.info.pop( + ROLLBACK_CAUSE_KEY) + + if isinstance( + dbe, exception.DBConnectionError): + context.is_disconnect = True raise def register_engine(engine): event.listen(engine, "handle_error", handler) + @event.listens_for(engine, "rollback_savepoint") + def rollback_savepoint(conn, name, context): + exc_info = sys.exc_info() + if exc_info[1]: + conn.info[ROLLBACK_CAUSE_KEY] = exc_info[1] + # NOTE(zzzeek) this eliminates a reference cycle between tracebacks + # that would occur in Python 3 only, which has been shown to occur if + # this function were in fact part of the traceback. That's not the + # case here however this is left as a defensive measure. + del exc_info + + # try to clear the "cause" ASAP outside of savepoints, + # by grabbing the end of transaction events... + @event.listens_for(engine, "rollback") + @event.listens_for(engine, "commit") + def pop_exc_tx(conn): + conn.info.pop(ROLLBACK_CAUSE_KEY, None) + + # .. as well as connection pool checkin (just in case). + # the .info dictionary lasts as long as the DBAPI connection itself + # and is cleared out when the connection is recycled or closed + # due to invalidate etc. + @event.listens_for(engine, "checkin") + def pop_exc_checkin(dbapi_conn, connection_record): + connection_record.info.pop(ROLLBACK_CAUSE_KEY, None) + def handle_connect_error(engine): """Connect to the engine, including handle_error handlers. diff --git a/oslo_db/tests/sqlalchemy/test_exc_filters.py b/oslo_db/tests/sqlalchemy/test_exc_filters.py index b353e69..65fb1b1 100644 --- a/oslo_db/tests/sqlalchemy/test_exc_filters.py +++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py @@ -23,6 +23,7 @@ import six import sqlalchemy as sqla from sqlalchemy import event import sqlalchemy.exc +from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import mapper from oslo_db import exception @@ -587,6 +588,73 @@ class TestReferenceErrorMySQL(TestReferenceErrorSQLite, self.assertEqual("resource_foo", matched.key_table) +class TestExceptionCauseMySQLSavepoint(test_base.MySQLOpportunisticTestCase): + def setUp(self): + super(TestExceptionCauseMySQLSavepoint, self).setUp() + + Base = declarative_base() + + class A(Base): + __tablename__ = 'a' + + id = sqla.Column(sqla.Integer, primary_key=True) + + __table_args__ = {'mysql_engine': 'InnoDB'} + + Base.metadata.create_all(self.engine) + + self.A = A + + def test_cause_for_failed_flush_plus_no_savepoint(self): + session = self.sessionmaker() + + with session.begin(): + session.add(self.A(id=1)) + try: + + with session.begin(): + + try: + with session.begin_nested(): + session.execute("rollback") + session.add(self.A(id=1)) + + # outermost is the failed SAVEPOINT rollback + # from the "with session.begin_nested()" + except exception.DBError as dbe_inner: + + # first "cause" is the failed SAVEPOINT rollback + # from inside of flush(), when it fails + self.assertTrue( + isinstance( + dbe_inner.cause, + exception.DBError + ) + ) + + # second "cause" is then the actual DB duplicate + self.assertTrue( + isinstance( + dbe_inner.cause.cause, + exception.DBDuplicateEntry + ) + ) + except exception.DBError as dbe_outer: + self.assertTrue( + isinstance( + dbe_outer.cause, + exception.DBDuplicateEntry + ) + ) + + # resets itself afterwards + try: + with session.begin(): + session.add(self.A(id=1)) + except exception.DBError as dbe_outer: + self.assertIsNone(dbe_outer.cause) + + class TestDBDataErrorSQLite(_SQLAExceptionMatcher, test_base.DbTestCase): def setUp(self): -- cgit v1.2.1