Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
Cloud:OpenStack:Newton:Staging
openstack-cinder
0002-short-circuit-notifications-when-not-enabl...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
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__)
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
