File 0002-short-circuit-notifications-when-not-enabled.patch of Package openstack-cinder
From a130387b4521a079e3408e8c8e3aa6c22caa8de2 Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Sat, 28 Jan 2017 19:18:17 +0100
Subject: [PATCH] Short-circuit notifications when not enabled
When running Cinder with notifications disabled we are still doing all
the work and calls to Oslo message notifier and it's there, at the very
last moment when it's going to send the message that it checks that
there's no actual extension loaded in the driver manager and skips the
actual send of the data.
This is considerably wasteful considering that for some of the
notifications we are actually querying the DB to get data, for example
volume attachments and glance metadata information when notifying about
volume usage.
This patch proposes short-circuiting notification methods as much as
possible to optimize code execution when Cinder has no notification
transport mechanism configured, as is the case when deployed as a
standalone SDS service.
Closes-Bug: #1660303
Change-Id: I77f655d3ef90088ce71304da5d4ea7b543991e90
---
diff --git a/cinder/api/contrib/qos_specs_manage.py b/cinder/api/contrib/qos_specs_manage.py
index f80f502..4f85bfa 100644
--- a/cinder/api/contrib/qos_specs_manage.py
+++ b/cinder/api/contrib/qos_specs_manage.py
@@ -47,6 +47,7 @@
_view_builder_class = view_qos_specs.ViewBuilder
@staticmethod
+ @utils.if_notifications_enabled
def _notify_qos_specs_error(context, method, payload):
rpc.get_notifier('QoSSpecs').error(context,
method,
diff --git a/cinder/api/contrib/types_manage.py b/cinder/api/contrib/types_manage.py
index 568030a..97ac8d2 100644
--- a/cinder/api/contrib/types_manage.py
+++ b/cinder/api/contrib/types_manage.py
@@ -39,12 +39,14 @@
_view_builder_class = views_types.ViewBuilder
+ @utils.if_notifications_enabled
def _notify_volume_type_error(self, context, method, err,
volume_type=None, id=None, name=None):
payload = dict(
volume_types=volume_type, name=name, id=id, error_message=err)
rpc.get_notifier('volumeType').error(context, method, payload)
+ @utils.if_notifications_enabled
def _notify_volume_type_info(self, context, method, volume_type):
payload = dict(volume_types=volume_type)
rpc.get_notifier('volumeType').info(context, method, payload)
diff --git a/cinder/api/v3/group_types.py b/cinder/api/v3/group_types.py
index e916abe..01f490b 100644
--- a/cinder/api/v3/group_types.py
+++ b/cinder/api/v3/group_types.py
@@ -42,12 +42,14 @@
}
policy.enforce(context, 'group:group_types_manage', target)
+ @utils.if_notifications_enabled
def _notify_group_type_error(self, context, method, err,
group_type=None, id=None, name=None):
payload = dict(
group_types=group_type, name=name, id=id, error_message=err)
rpc.get_notifier('groupType').error(context, method, payload)
+ @utils.if_notifications_enabled
def _notify_group_type_info(self, context, method, group_type):
payload = dict(group_types=group_type)
rpc.get_notifier('groupType').info(context, method, payload)
diff --git a/cinder/image/cache.py b/cinder/image/cache.py
index 46a5313..c24b717 100644
--- a/cinder/image/cache.py
+++ b/cinder/image/cache.py
@@ -22,6 +22,7 @@
from cinder.i18n import _LW
from cinder import objects
from cinder import rpc
+from cinder import utils
CONF = cfg.CONF
@@ -182,15 +183,19 @@
return True
+ @utils.if_notifications_enabled
def _notify_cache_hit(self, context, image_id, host):
self._notify_cache_action(context, image_id, host, 'hit')
+ @utils.if_notifications_enabled
def _notify_cache_miss(self, context, image_id, host):
self._notify_cache_action(context, image_id, host, 'miss')
+ @utils.if_notifications_enabled
def _notify_cache_eviction(self, context, image_id, host):
self._notify_cache_action(context, image_id, host, 'evict')
+ @utils.if_notifications_enabled
def _notify_cache_action(self, context, image_id, host, action):
data = {
'image_id': image_id,
diff --git a/cinder/rpc.py b/cinder/rpc.py
index ab4d032..b59d232 100644
--- a/cinder/rpc.py
+++ b/cinder/rpc.py
@@ -38,6 +38,7 @@
from cinder.i18n import _LE, _LI
from cinder import objects
from cinder.objects import base
+from cinder import utils
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
@@ -74,9 +75,15 @@
allowed_remote_exmods=exmods,
aliases=TRANSPORT_ALIASES)
- serializer = RequestContextSerializer(JsonPayloadSerializer())
- NOTIFIER = messaging.Notifier(NOTIFICATION_TRANSPORT,
- serializer=serializer)
+ # get_notification_transport has loaded oslo_messaging_notifications config
+ # group, so we can now check if notifications are actually enabled.
+ if conf.oslo_messaging_notifications.transport_url:
+ json_serializer = JsonPayloadSerializer()
+ serializer = RequestContextSerializer(json_serializer)
+ NOTIFIER = messaging.Notifier(NOTIFICATION_TRANSPORT,
+ serializer=serializer)
+ else:
+ NOTIFIER = utils.DO_NOTHING
def initialized():
@@ -164,6 +171,7 @@
serializer=serializer)
+@utils.if_notifications_enabled
def get_notifier(service=None, host=None, publisher_id=None):
assert NOTIFIER is not None
if not publisher_id:
diff --git a/cinder/scheduler/flows/create_volume.py b/cinder/scheduler/flows/create_volume.py
index eeb048f..7cb8e4d 100644
--- a/cinder/scheduler/flows/create_volume.py
+++ b/cinder/scheduler/flows/create_volume.py
@@ -99,6 +99,7 @@
LOG.error(_LE("Failed to run task %(name)s: %(cause)s"),
{'cause': cause, 'name': self.name})
+ @utils.if_notifications_enabled
def _notify_failure(self, context, request_spec, cause):
"""When scheduling fails send out an event that it failed."""
payload = {
diff --git a/cinder/test.py b/cinder/test.py
index 2ad6210..eb98350 100644
--- a/cinder/test.py
+++ b/cinder/test.py
@@ -50,6 +50,7 @@
from cinder.tests import fixtures as cinder_fixtures
from cinder.tests.unit import conf_fixture
from cinder.tests.unit import fake_notifier
+from cinder.tests.unit import utils as test_utils
CONF = cfg.CONF
@@ -223,6 +224,8 @@
coordination.COORDINATOR.start()
self.addCleanup(coordination.COORDINATOR.stop)
+ test_utils.set_normal_rpc_notifier(self)
+
def _restore_obj_registry(self):
objects_base.CinderObjectRegistry._registry._obj_classes = \
self._base_test_obj_backup
diff --git a/cinder/tests/unit/fake_notifier.py b/cinder/tests/unit/fake_notifier.py
index 312a474..7e07a26 100644
--- a/cinder/tests/unit/fake_notifier.py
+++ b/cinder/tests/unit/fake_notifier.py
@@ -28,8 +28,8 @@
class FakeNotifier(object):
- def __init__(self, transport, publisher_id, serializer=None, driver=None,
- topic=None, retry=None):
+ def __init__(self, transport, publisher_id=None, serializer=None,
+ driver=None, topic=None, retry=None):
self.transport = transport
self.publisher_id = publisher_id
for priority in ['debug', 'info', 'warn', 'error', 'critical']:
diff --git a/cinder/tests/unit/test_rpc.py b/cinder/tests/unit/test_rpc.py
index 4fa43fd..39ef198 100644
--- a/cinder/tests/unit/test_rpc.py
+++ b/cinder/tests/unit/test_rpc.py
@@ -83,3 +83,18 @@
self.assertFalse(get_min_obj.called)
self.assertFalse(get_min_rpc.called)
+
+ @mock.patch('oslo_messaging.JsonPayloadSerializer', wraps=True)
+ def test_init_no_notifications(self, serializer_mock):
+ self.override_config('transport_url', '',
+ group='oslo_messaging_notifications')
+ rpc.init(test.CONF)
+ self.assertEqual(rpc.utils.DO_NOTHING, rpc.NOTIFIER)
+ serializer_mock.assert_not_called()
+
+ @mock.patch.object(rpc, 'messaging')
+ def test_init_notifications(self, messaging_mock):
+ rpc.init(test.CONF)
+ self.assertTrue(messaging_mock.JsonPayloadSerializer.called)
+ self.assertTrue(messaging_mock.Notifier.called)
+ self.assertEqual(rpc.NOTIFIER, messaging_mock.Notifier.return_value)
diff --git a/cinder/tests/unit/test_utils.py b/cinder/tests/unit/test_utils.py
index 166f405..788f1e7 100644
--- a/cinder/tests/unit/test_utils.py
+++ b/cinder/tests/unit/test_utils.py
@@ -16,4 +16,5 @@
import datetime
import functools
+import json
import os
import time
@@ -1413,3 +1414,36 @@
self.assertRaises(webob.exc.HTTPBadRequest,
utils.validate_integer,
value, 'limit', min_value=-1, max_value=(2 ** 31))
+
+
+class TestNotificationShortCircuit(test.TestCase):
+ def test_do_nothing_getter(self):
+ """Test any attribute will always return the same instance (self)."""
+ donothing = utils.DoNothing()
+ self.assertIs(donothing, donothing.anyname)
+
+ def test_do_nothing_caller(self):
+ """Test calling the object will always return the same instance."""
+ donothing = utils.DoNothing()
+ self.assertIs(donothing, donothing())
+
+ def test_do_nothing_json_serializable(self):
+ """Test calling the object will always return the same instance."""
+ donothing = utils.DoNothing()
+ self.assertEqual('""', json.dumps(donothing))
+
+ @utils.if_notifications_enabled
+ def _decorated_method(self):
+ return mock.sentinel.success
+
+ def test_if_notification_enabled_when_enabled(self):
+ """Test method is called when notifications are enabled."""
+ result = self._decorated_method()
+ self.assertEqual(mock.sentinel.success, result)
+
+ def test_if_notification_enabled_when_disabled(self):
+ """Test method is not called when notifications are disabled."""
+ self.override_config('transport_url', '',
+ group='oslo_messaging_notifications')
+ result = self._decorated_method()
+ self.assertEqual(utils.DO_NOTHING, result)
diff --git a/cinder/tests/unit/utils.py b/cinder/tests/unit/utils.py
index b756071..b96cfd7 100644
--- a/cinder/tests/unit/utils.py
+++ b/cinder/tests/unit/utils.py
@@ -27,6 +27,7 @@ from cinder import db
from cinder import exception
from cinder import objects
from cinder.objects import fields
+from cinder import rpc
from cinder.tests.unit import fake_constants as fake
@@ -436,3 +437,13 @@ def generate_timeout_series(timeout):
while True:
iteration += 1
yield (iteration * timeout) + iteration
+
+
+def set_normal_rpc_notifier(test_case):
+ """Instead of shortcircuiting notifications, user Oslo's notifier."""
+ test_case.override_config('transport_url', 'fake_transport',
+ group='oslo_messaging_notifications')
+ json_serializer = rpc.messaging.JsonPayloadSerializer()
+ serializer = rpc.RequestContextSerializer(json_serializer)
+ rpc.NOTIFIER = rpc.messaging.Notifier(rpc.NOTIFICATION_TRANSPORT,
+ serializer=serializer)
diff --git a/cinder/utils.py b/cinder/utils.py
index 59ce3b2..2d79f0b 100644
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -1072,3 +1072,28 @@
def service_expired_time(with_timezone=False):
return (timeutils.utcnow(with_timezone=with_timezone) -
datetime.timedelta(seconds=CONF.service_down_time))
+
+
+class DoNothing(str):
+ """Class that literrally does nothing.
+
+ We inherit from str in case it's called with json.dumps.
+ """
+ def __call__(self, *args, **kwargs):
+ return self
+
+ def __getattr__(self, name):
+ return self
+
+
+DO_NOTHING = DoNothing()
+
+
+def if_notifications_enabled(f):
+ """Calls decorated method only if notifications are enabled."""
+ @functools.wraps(f)
+ def wrapped(*args, **kwargs):
+ if CONF.oslo_messaging_notifications.transport_url:
+ return f(*args, **kwargs)
+ return DO_NOTHING
+ return wrapped
diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py
index f86b6bc..a607395 100644
--- a/cinder/volume/utils.py
+++ b/cinder/volume/utils.py
@@ -123,6 +123,7 @@
return usage_info
+@utils.if_notifications_enabled
def notify_about_volume_usage(context, volume, event_suffix,
extra_usage_info=None, host=None):
if not host:
@@ -137,6 +138,7 @@
usage_info)
+@utils.if_notifications_enabled
def notify_about_backup_usage(context, backup, event_suffix,
extra_usage_info=None,
host=None):
@@ -183,6 +185,7 @@
return usage_info
+@utils.if_notifications_enabled
def notify_about_snapshot_usage(context, snapshot, event_suffix,
extra_usage_info=None, host=None):
if not host:
@@ -229,6 +233,7 @@
usage_info)
+@utils.if_notifications_enabled
def notify_about_replication_usage(context, volume, suffix,
extra_usage_info=None, host=None):
if not host:
@@ -245,6 +250,7 @@
usage_info)
+@utils.if_notifications_enabled
def notify_about_replication_error(context, volume, suffix,
extra_error_info=None, host=None):
if not host:
@@ -274,6 +280,7 @@
return usage_info
+@utils.if_notifications_enabled
def notify_about_consistencygroup_usage(context, group, event_suffix,
extra_usage_info=None, host=None):
if not host:
@@ -305,6 +312,7 @@
return usage_info
+@utils.if_notifications_enabled
def notify_about_group_usage(context, group, event_suffix,
extra_usage_info=None, host=None):
if not host:
@@ -351,6 +359,7 @@
return usage_info
+@utils.if_notifications_enabled
def notify_about_cgsnapshot_usage(context, cgsnapshot, event_suffix,
extra_usage_info=None, host=None):
if not host:
@@ -368,6 +377,7 @@
usage_info)
+@utils.if_notifications_enabled
def notify_about_group_snapshot_usage(context, group_snapshot, event_suffix,
extra_usage_info=None, host=None):
if not host:
diff --git a/cinder/volume/volume_types.py b/cinder/volume/volume_types.py
index e688cbf..ddba3f1 100644
--- a/cinder/volume/volume_types.py
+++ b/cinder/volume/volume_types.py
@@ -31,5 +31,6 @@
from cinder.i18n import _, _LE
from cinder import quota
+from cinder import utils
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
