Put service callback retries on a different queue

At the moment, if a service callback fails, it will get put on the retry queue.
This causes a potential problem though:

If a service's callback server goes down, we may generate a lot of retries and
this may then put a lot of items on the retry queue. The retry queue is also
responsible for other important parts of Notify such as retrying message
delivery and we don't want a service's callback server going down to have an
impact on the rest of Notify.

Putting the retries on a different queue means that tasks get processed
faster than if they were put back on the same 'service-callbacks' queue.
This commit is contained in:
Katie Smith
2021-02-04 11:53:22 +00:00
parent e0ddb5a39e
commit 5eebcf6452
5 changed files with 7 additions and 4 deletions

View File

@@ -92,7 +92,7 @@ def _send_data_to_service_callback_api(self, data, service_callback_url, token,
) )
if not isinstance(e, HTTPError) or e.response.status_code >= 500: if not isinstance(e, HTTPError) or e.response.status_code >= 500:
try: try:
self.retry(queue=QueueNames.RETRY) self.retry(queue=QueueNames.CALLBACKS_RETRY)
except self.MaxRetriesExceededError: except self.MaxRetriesExceededError:
current_app.logger.warning( current_app.logger.warning(
"Retry: {} has retried the max num of times for callback url {} and notification_id: {}".format( "Retry: {} has retried the max num of times for callback url {} and notification_id: {}".format(

View File

@@ -27,6 +27,7 @@ class QueueNames(object):
PROCESS_FTP = 'process-ftp-tasks' PROCESS_FTP = 'process-ftp-tasks'
CREATE_LETTERS_PDF = 'create-letters-pdf-tasks' CREATE_LETTERS_PDF = 'create-letters-pdf-tasks'
CALLBACKS = 'service-callbacks' CALLBACKS = 'service-callbacks'
CALLBACKS_RETRY = 'service-callbacks-retry'
LETTERS = 'letter-tasks' LETTERS = 'letter-tasks'
SMS_CALLBACKS = 'sms-callbacks' SMS_CALLBACKS = 'sms-callbacks'
ANTIVIRUS = 'antivirus-tasks' ANTIVIRUS = 'antivirus-tasks'
@@ -50,6 +51,7 @@ class QueueNames(object):
QueueNames.NOTIFY, QueueNames.NOTIFY,
QueueNames.CREATE_LETTERS_PDF, QueueNames.CREATE_LETTERS_PDF,
QueueNames.CALLBACKS, QueueNames.CALLBACKS,
QueueNames.CALLBACKS_RETRY,
QueueNames.LETTERS, QueueNames.LETTERS,
QueueNames.SMS_CALLBACKS, QueueNames.SMS_CALLBACKS,
QueueNames.SAVE_API_EMAIL, QueueNames.SAVE_API_EMAIL,

View File

@@ -51,7 +51,7 @@ case $NOTIFY_APP_NAME in
;; ;;
delivery-worker-service-callbacks) delivery-worker-service-callbacks)
exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \ exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \
-Q service-callbacks 2> /dev/null -Q service-callbacks,service-callbacks-retry 2> /dev/null
;; ;;
delivery-worker-save-api-notifications) delivery-worker-save-api-notifications)
exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \ exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \

View File

@@ -109,7 +109,7 @@ def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_
send_delivery_status_to_service(notification.id, encrypted_status_update=encrypted_data) send_delivery_status_to_service(notification.id, encrypted_status_update=encrypted_data)
assert mocked.call_count == 1 assert mocked.call_count == 1
assert mocked.call_args[1]['queue'] == 'retry-tasks' assert mocked.call_args[1]['queue'] == 'service-callbacks-retry'
@pytest.mark.parametrize("notification_type", @pytest.mark.parametrize("notification_type",

View File

@@ -60,7 +60,7 @@ def test_load_config_if_cloudfoundry_not_available(reload_config):
def test_queue_names_all_queues_correct(): def test_queue_names_all_queues_correct():
# Need to ensure that all_queues() only returns queue names used in API # Need to ensure that all_queues() only returns queue names used in API
queues = QueueNames.all_queues() queues = QueueNames.all_queues()
assert len(queues) == 17 assert len(queues) == 18
assert set([ assert set([
QueueNames.PRIORITY, QueueNames.PRIORITY,
QueueNames.PERIODIC, QueueNames.PERIODIC,
@@ -74,6 +74,7 @@ def test_queue_names_all_queues_correct():
QueueNames.NOTIFY, QueueNames.NOTIFY,
QueueNames.CREATE_LETTERS_PDF, QueueNames.CREATE_LETTERS_PDF,
QueueNames.CALLBACKS, QueueNames.CALLBACKS,
QueueNames.CALLBACKS_RETRY,
QueueNames.LETTERS, QueueNames.LETTERS,
QueueNames.SMS_CALLBACKS, QueueNames.SMS_CALLBACKS,
QueueNames.SAVE_API_EMAIL, QueueNames.SAVE_API_EMAIL,