From 7e69891412e967a8509fe6877501114e148f7518 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Tue, 2 Aug 2016 15:51:18 -0700 Subject: [PATCH] ml2: allow retry on retriabable db error by precommit Db retriable error by precommit can be recovered with upper layer by retry instead of converting into ML2MechanismDriverError. Raise retriable db error instead of swallowing it and let upper layer to retry. Due to concurrency, db error by precommit is sometimes inevitable. precommit may issue db transactions depending on mechanism driver, Some operations on e.g. subnet, routers, touches many other resources (e.g. ip allocation, ports) in single db transaction as well. And background operation by mechanism driver (e.g. background sync, monitoring resources periodically, etc) may touch those db tables with other order. Change-Id: Ifc670c1eb5801934bf46fdc23a7721ce4c2cfa6c Closes-Bug: #1609184 Closes-Bug: #1609149 --- neutron/plugins/ml2/managers.py | 81 ++++++++++++++----- .../tests/unit/plugins/ml2/test_managers.py | 41 ++++++++++ 2 files changed, 101 insertions(+), 21 deletions(-) diff --git a/neutron/plugins/ml2/managers.py b/neutron/plugins/ml2/managers.py index 12fcaee93cd..d24fb319fef 100644 --- a/neutron/plugins/ml2/managers.py +++ b/neutron/plugins/ml2/managers.py @@ -23,6 +23,7 @@ import six import stevedore from neutron._i18n import _, _LE, _LI, _LW +from neutron.db import api as db_api from neutron.db import segments_db from neutron.extensions import external_net from neutron.extensions import multiprovidernet as mpnet @@ -388,21 +389,32 @@ class MechanismManager(stevedore.named.NamedExtensionManager): raise vlantransparent.VlanTransparencyDriverError() def _call_on_drivers(self, method_name, context, - continue_on_failure=False): + continue_on_failure=False, raise_db_retriable=False): """Helper method for calling a method across all mechanism drivers. :param method_name: name of the method to call :param context: context parameter to pass to each method call :param continue_on_failure: whether or not to continue to call all mechanism drivers once one has raised an exception + :param raise_db_retriable: whether or not to treat retriable db + exception by mechanism drivers to propagate up to upper layer so + that upper layer can handle it or error in ML2 player :raises: neutron.plugins.ml2.common.MechanismDriverError - if any mechanism driver call fails. + if any mechanism driver call fails. or DB retriable error when + raise_db_retriable=False. See neutron.db.api.is_retriable for + what db exception is retriable """ error = False for driver in self.ordered_mech_drivers: try: getattr(driver.obj, method_name)(context) - except Exception: + except Exception as e: + if raise_db_retriable and db_api.is_retriable(e): + with excutils.save_and_reraise_exception(): + LOG.debug("DB exception raised by Mechanism driver " + "'%(name)s' in %(method)s", + {'name': driver.name, 'method': method_name}, + exc_info=e) LOG.exception( _LE("Mechanism driver '%(name)s' failed in %(method)s"), {'name': driver.name, 'method': method_name} @@ -418,7 +430,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def create_network_precommit(self, context): """Notify all mechanism drivers during network creation. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises: DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver create_network_precommit call fails. Called within the database transaction. If a mechanism driver @@ -427,7 +441,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): that all mechanism drivers are called in this case. """ self._check_vlan_transparency(context) - self._call_on_drivers("create_network_precommit", context) + self._call_on_drivers("create_network_precommit", context, + raise_db_retriable=True) def create_network_postcommit(self, context): """Notify all mechanism drivers after network creation. @@ -446,7 +461,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def update_network_precommit(self, context): """Notify all mechanism drivers during network update. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises: DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver update_network_precommit call fails. Called within the database transaction. If a mechanism driver @@ -454,7 +471,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): to the caller, triggering a rollback. There is no guarantee that all mechanism drivers are called in this case. """ - self._call_on_drivers("update_network_precommit", context) + self._call_on_drivers("update_network_precommit", context, + raise_db_retriable=True) def update_network_postcommit(self, context): """Notify all mechanism drivers after network update. @@ -473,7 +491,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def delete_network_precommit(self, context): """Notify all mechanism drivers during network deletion. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises: DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver delete_network_precommit call fails. Called within the database transaction. If a mechanism driver @@ -481,7 +501,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): to the caller, triggering a rollback. There is no guarantee that all mechanism drivers are called in this case. """ - self._call_on_drivers("delete_network_precommit", context) + self._call_on_drivers("delete_network_precommit", context, + raise_db_retriable=True) def delete_network_postcommit(self, context): """Notify all mechanism drivers after network deletion. @@ -504,7 +525,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def create_subnet_precommit(self, context): """Notify all mechanism drivers during subnet creation. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises: DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver create_subnet_precommit call fails. Called within the database transaction. If a mechanism driver @@ -512,7 +535,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): to the caller, triggering a rollback. There is no guarantee that all mechanism drivers are called in this case. """ - self._call_on_drivers("create_subnet_precommit", context) + self._call_on_drivers("create_subnet_precommit", context, + raise_db_retriable=True) def create_subnet_postcommit(self, context): """Notify all mechanism drivers after subnet creation. @@ -531,7 +555,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def update_subnet_precommit(self, context): """Notify all mechanism drivers during subnet update. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises: DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver update_subnet_precommit call fails. Called within the database transaction. If a mechanism driver @@ -539,7 +565,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): to the caller, triggering a rollback. There is no guarantee that all mechanism drivers are called in this case. """ - self._call_on_drivers("update_subnet_precommit", context) + self._call_on_drivers("update_subnet_precommit", context, + raise_db_retriable=True) def update_subnet_postcommit(self, context): """Notify all mechanism drivers after subnet update. @@ -558,7 +585,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def delete_subnet_precommit(self, context): """Notify all mechanism drivers during subnet deletion. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises: DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver delete_subnet_precommit call fails. Called within the database transaction. If a mechanism driver @@ -566,7 +595,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): to the caller, triggering a rollback. There is no guarantee that all mechanism drivers are called in this case. """ - self._call_on_drivers("delete_subnet_precommit", context) + self._call_on_drivers("delete_subnet_precommit", context, + raise_db_retriable=True) def delete_subnet_postcommit(self, context): """Notify all mechanism drivers after subnet deletion. @@ -589,7 +619,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def create_port_precommit(self, context): """Notify all mechanism drivers during port creation. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises: DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver create_port_precommit call fails. Called within the database transaction. If a mechanism driver @@ -597,7 +629,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): to the caller, triggering a rollback. There is no guarantee that all mechanism drivers are called in this case. """ - self._call_on_drivers("create_port_precommit", context) + self._call_on_drivers("create_port_precommit", context, + raise_db_retriable=True) def create_port_postcommit(self, context): """Notify all mechanism drivers of port creation. @@ -616,7 +649,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def update_port_precommit(self, context): """Notify all mechanism drivers during port update. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises: DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver update_port_precommit call fails. Called within the database transaction. If a mechanism driver @@ -624,7 +659,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): to the caller, triggering a rollback. There is no guarantee that all mechanism drivers are called in this case. """ - self._call_on_drivers("update_port_precommit", context) + self._call_on_drivers("update_port_precommit", context, + raise_db_retriable=True) def update_port_postcommit(self, context): """Notify all mechanism drivers after port update. @@ -643,7 +679,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): def delete_port_precommit(self, context): """Notify all mechanism drivers during port deletion. - :raises: neutron.plugins.ml2.common.MechanismDriverError + :raises:DB retriable error if create_network_precommit raises them + See neutron.db.api.is_retriable for what db exception is retriable + or neutron.plugins.ml2.common.MechanismDriverError if any mechanism driver delete_port_precommit call fails. Called within the database transaction. If a mechanism driver @@ -651,7 +689,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager): to the caller, triggering a rollback. There is no guarantee that all mechanism drivers are called in this case. """ - self._call_on_drivers("delete_port_precommit", context) + self._call_on_drivers("delete_port_precommit", context, + raise_db_retriable=True) def delete_port_postcommit(self, context): """Notify all mechanism drivers after port deletion. diff --git a/neutron/tests/unit/plugins/ml2/test_managers.py b/neutron/tests/unit/plugins/ml2/test_managers.py index b244b60ecc4..c9e3557f06e 100644 --- a/neutron/tests/unit/plugins/ml2/test_managers.py +++ b/neutron/tests/unit/plugins/ml2/test_managers.py @@ -16,9 +16,14 @@ import mock +from oslo_db import exception as db_exc + +from neutron.plugins.ml2.common import exceptions as ml2_exc +from neutron.plugins.ml2 import config as config from neutron.plugins.ml2 import driver_api as api from neutron.plugins.ml2 import managers from neutron.tests import base +from neutron.tests.unit.plugins.ml2.drivers import mechanism_test class TestManagers(base.BaseTestCase): @@ -36,3 +41,39 @@ class TestManagers(base.BaseTestCase): bindinglevel.segment_id = 'fake_seg_id1' self.assertTrue(manager._check_driver_to_bind( 'fake_driver', segments_to_bind, binding_levels)) + + +class TestMechManager(base.BaseTestCase): + def setUp(self): + config.cfg.CONF.set_override('mechanism_drivers', ['test'], + group='ml2') + super(TestMechManager, self).setUp() + self._manager = managers.MechanismManager() + + def _check_precommit(self, resource, operation): + meth_name = "%s_%s_precommit" % (operation, resource) + method = getattr(self._manager, meth_name) + fake_ctxt = mock.Mock() + fake_ctxt.current = {} + + with mock.patch.object(mechanism_test.TestMechanismDriver, meth_name, + side_effect=db_exc.DBDeadlock()): + self.assertRaises(db_exc.DBDeadlock, method, fake_ctxt) + + with mock.patch.object(mechanism_test.TestMechanismDriver, meth_name, + side_effect=RuntimeError()): + self.assertRaises(ml2_exc.MechanismDriverError, method, fake_ctxt) + + def _check_resource(self, resource): + self._check_precommit(resource, 'create') + self._check_precommit(resource, 'update') + self._check_precommit(resource, 'delete') + + def test_network_precommit(self): + self._check_resource('network') + + def test_subnet_precommit(self): + self._check_resource('subnet') + + def test_port_precommit(self): + self._check_resource('port')