From 143b19c8d5e6805c4ce0abe78748c0add42a2072 Mon Sep 17 00:00:00 2001
From: Kevin Benton <kevin@benton.pub>
Date: Tue, 7 Jun 2016 15:27:48 -0700
Subject: [PATCH] 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                 |  2 ++
 neutron/db/api.py                            | 16 ++++++++++++++++
 neutron/tests/unit/callbacks/test_manager.py | 11 +++++++++++
 3 files changed, 29 insertions(+)

diff --git a/neutron/callbacks/manager.py b/neutron/callbacks/manager.py
index 0b16d7004b2..22159959994 100644
--- a/neutron/callbacks/manager.py
+++ b/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).
 
diff --git a/neutron/db/api.py b/neutron/db/api.py
index 6f658ea94c3..31269e0d1d5 100644
--- a/neutron/db/api.py
+++ b/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
diff --git a/neutron/tests/unit/callbacks/test_manager.py b/neutron/tests/unit/callbacks/test_manager.py
index 69a39bdd2e2..92809db9d19 100644
--- a/neutron/tests/unit/callbacks/test_manager.py
+++ b/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