Separate exception class for retriables in callbacks

There are various places where we emit a callback to allow
subscribers to do validation or additional bookkeeping during
the various life-cycle events of objects. However, a subscriber
can encounter a DB Deadlock, StaleDataError or other retriable
error that just needs the transaction to be restarted. In the
rest of the Neutron code these exceptions bubble up to the API
layer where the whole request is restarted. However, the exception
catching for the callbacks in various places was preventing these
errors from being retried because they were lost in conversion.

This patch has the callback manager convert retriable exceptions
into RetryRequest exceptions that will not be caught by handlers
of CallbackError so by default it will bubble up to the API layer
and be retried. 'notify' callers can avoid this behavior by
additionally catching RetryRequest.

Closes-Bug: #1590316
Change-Id: I6732c60f89de4318b5f56327c5bc966bd250baae

neutron/callbacks/manager.py

@@ -18,6 +18,7 @@ from oslo_utils import reflection
 from neutron._i18n import _LE
 from neutron.callbacks import events
 from neutron.callbacks import exceptions
+from neutron.db import api as db_api
 
 LOG = logging.getLogger(__name__)
 
@@ -106,6 +107,7 @@ class CallbacksManager(object):
                     del self._callbacks[resource][event][callback_id]
             del self._index[callback_id]
 
+    @db_api.reraise_as_retryrequest
     def notify(self, resource, event, trigger, **kwargs):
         """Notify all subscribed callback(s).
 

neutron/db/api.py

@@ -22,6 +22,7 @@ from oslo_db import exception as db_exc
 from oslo_db.sqlalchemy import enginefacade
 from oslo_utils import excutils
 import osprofiler.sqlalchemy
+import six
 import sqlalchemy
 from sqlalchemy.orm import exc
 
@@ -54,6 +55,21 @@ retry_db_errors = oslo_db_api.wrap_db_retry(
 )
 
 
+def reraise_as_retryrequest(f):
+    """Packs retriable exceptions into a RetryRequest."""
+
+    @six.wraps(f)
+    def wrapped(*args, **kwargs):
+        try:
+            return f(*args, **kwargs)
+        except Exception as e:
+            with excutils.save_and_reraise_exception() as ctx:
+                if is_retriable(e):
+                    ctx.reraise = False
+                    raise db_exc.RetryRequest(e)
+    return wrapped
+
+
 def _is_nested_instance(e, etypes):
     """Check if exception or its inner excepts are an instance of etypes."""
     return (isinstance(e, etypes) or

neutron/tests/unit/callbacks/test_manager.py

@@ -13,6 +13,7 @@
 #    under the License.
 
 import mock
+from oslo_db import exception as db_exc
 
 from neutron.callbacks import events
 from neutron.callbacks import exceptions
@@ -35,6 +36,10 @@ def callback_raise(*args, **kwargs):
     raise Exception()
 
 
+def callback_raise_retriable(*args, **kwargs):
+    raise db_exc.DBDeadlock()
+
+
 class CallBacksManagerTestCase(base.BaseTestCase):
 
     def setUp(self):
@@ -173,6 +178,12 @@ class CallBacksManagerTestCase(base.BaseTestCase):
                               resources.PORT, events.BEFORE_CREATE, self)
         self.assertIsInstance(e.errors[0], exceptions.NotificationError)
 
+    def test_notify_handle_retriable_exception(self):
+        self.manager.subscribe(
+            callback_raise_retriable, resources.PORT, events.BEFORE_CREATE)
+        self.assertRaises(db_exc.RetryRequest, self.manager.notify,
+                          resources.PORT, events.BEFORE_CREATE, self)
+
     def test_notify_called_once_with_no_failures(self):
         with mock.patch.object(self.manager, '_notify_loop') as n:
             n.return_value = False