Fix wrong logic in notification error handling
Notifications are sent indefinitely to the error queue instead of being sent again on info queue. Change-Id: Ie8e946fdbe8a6c977a869fb4d58fb764effc2c35
This commit is contained in:
parent
4fbb9d7b56
commit
04c4fcf900
|
@ -12,9 +12,10 @@
|
||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
from oslo_log import log as logging
|
|
||||||
import time
|
import time
|
||||||
|
|
||||||
|
from oslo_log import log as logging
|
||||||
|
|
||||||
LOG = logging.getLogger(__name__)
|
LOG = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
@ -66,27 +67,35 @@ class NotificationHandler(object):
|
||||||
LOG.warning('Notification payload: %s', notification.payload)
|
LOG.warning('Notification payload: %s', notification.payload)
|
||||||
LOG.warning('Notification metadata: %s', notification.metadata)
|
LOG.warning('Notification metadata: %s', notification.metadata)
|
||||||
LOG.exception(e)
|
LOG.exception(e)
|
||||||
notification.increment_retry_count()
|
self._send_notification_to_error_queue(notification)
|
||||||
self._retry_notification(notification)
|
|
||||||
|
|
||||||
def error(self, ctxt, publisher_id, event_type, payload, metadata):
|
def error(self, ctxt, publisher_id, event_type, payload, metadata):
|
||||||
LOG.warning('Received event "%s" from "%s" on error queue', event_type, publisher_id)
|
LOG.warning('Received event "%s" from "%s" on error queue', event_type, publisher_id)
|
||||||
notification = NotificationMessage(event_type, ctxt, payload, metadata)
|
notification = NotificationMessage(event_type, ctxt, payload, metadata)
|
||||||
|
|
||||||
if notification.has_counter():
|
if notification.has_counter():
|
||||||
LOG.error('Retry notification "%s" on info queue', event_type)
|
self._send_notification_to_info_queue(notification)
|
||||||
time.sleep(self.config.collector.retry_delay)
|
|
||||||
self._retry_notification(notification)
|
|
||||||
else:
|
else:
|
||||||
LOG.info('Ignoring notification "%s" sent on error queue', event_type)
|
LOG.info('Ignoring notification "%s" sent on error queue', event_type)
|
||||||
|
|
||||||
def _retry_notification(self, notification):
|
def _send_notification_to_error_queue(self, notification):
|
||||||
notifier = self.messaging.get_notifier()
|
notifier = self.messaging.get_notifier()
|
||||||
|
notification.increment_retry_count()
|
||||||
|
|
||||||
if notification.get_retry_counter() >= self.config.collector.max_retries:
|
if notification.get_retry_counter() > self.config.collector.max_retries:
|
||||||
LOG.critical('Send notification "%s" (%s) to critical queue',
|
LOG.critical('Send notification "%s" (%s) to critical queue',
|
||||||
notification.metadata.get('message_id'),
|
notification.metadata.get('message_id'),
|
||||||
notification.event_type)
|
notification.event_type)
|
||||||
notifier.critical(notification.context, notification.event_type, notification.payload)
|
notifier.critical(notification.context, notification.event_type, notification.payload)
|
||||||
else:
|
else:
|
||||||
notifier.error(notification.context, notification.event_type, notification.payload)
|
notifier.error(notification.context, notification.event_type, notification.payload)
|
||||||
|
|
||||||
|
def _send_notification_to_info_queue(self, notification):
|
||||||
|
LOG.error('Retry notification "%s" on info queue in %s seconds',
|
||||||
|
notification.event_type,
|
||||||
|
self.config.collector.retry_delay)
|
||||||
|
|
||||||
|
time.sleep(self.config.collector.retry_delay)
|
||||||
|
|
||||||
|
notifier = self.messaging.get_notifier()
|
||||||
|
notifier.info(notification.context, notification.event_type, notification.payload)
|
||||||
|
|
|
@ -63,13 +63,18 @@ class TestNotification(base.BaseTestCase):
|
||||||
|
|
||||||
self.notifier.error.assert_called_once()
|
self.notifier.error.assert_called_once()
|
||||||
|
|
||||||
def test_notifications_are_sent_again_to_error_queue_if_under_threshold(self):
|
self.notifier.info.assert_not_called()
|
||||||
|
self.notifier.critical.assert_not_called()
|
||||||
|
|
||||||
|
def test_notifications_are_sent_again_to_info_queue_if_under_threshold(self):
|
||||||
context = {
|
context = {
|
||||||
notification.NotificationMessage.RETRY_COUNTER: 2
|
notification.NotificationMessage.RETRY_COUNTER: 2
|
||||||
}
|
}
|
||||||
|
|
||||||
self.handler.error(context, 'compute.nova01', 'some_event', dict(), dict())
|
self.handler.error(context, 'compute.nova01', 'some_event', dict(), dict())
|
||||||
self.notifier.error.assert_called_once()
|
self.notifier.info.assert_called_once()
|
||||||
|
|
||||||
|
self.notifier.error.assert_not_called()
|
||||||
self.notifier.critical.assert_not_called()
|
self.notifier.critical.assert_not_called()
|
||||||
|
|
||||||
def test_notifications_are_sent_to_critical_queue_if_above_threshold(self):
|
def test_notifications_are_sent_to_critical_queue_if_above_threshold(self):
|
||||||
|
@ -77,11 +82,19 @@ class TestNotification(base.BaseTestCase):
|
||||||
notification.NotificationMessage.RETRY_COUNTER: 3
|
notification.NotificationMessage.RETRY_COUNTER: 3
|
||||||
}
|
}
|
||||||
|
|
||||||
self.handler.error(context, 'compute.nova01', 'some_event', dict(), dict())
|
event_handler = mock.Mock()
|
||||||
self.notifier.error.assert_not_called()
|
event_handler.handle_events.side_effect = Exception()
|
||||||
|
|
||||||
|
self.handler.add_event_handler(event_handler)
|
||||||
|
|
||||||
|
self.handler.info(context, 'compute.nova01', 'some_event', dict(), dict())
|
||||||
self.notifier.critical.assert_called_once()
|
self.notifier.critical.assert_called_once()
|
||||||
|
|
||||||
|
self.notifier.info.assert_not_called()
|
||||||
|
self.notifier.error.assert_not_called()
|
||||||
|
|
||||||
def test_unrelated_notifications_are_not_handled_in_error_queue(self):
|
def test_unrelated_notifications_are_not_handled_in_error_queue(self):
|
||||||
self.handler.error(dict(), 'compute.nova01', 'some_event', dict(), dict())
|
self.handler.error(dict(), 'compute.nova01', 'some_event', dict(), dict())
|
||||||
|
self.notifier.info.assert_not_called()
|
||||||
self.notifier.error.assert_not_called()
|
self.notifier.error.assert_not_called()
|
||||||
self.notifier.critical.assert_not_called()
|
self.notifier.critical.assert_not_called()
|
||||||
|
|
Loading…
Reference in New Issue