From 5eebcf64525a707bed19f191e86d69685be907fd Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 4 Feb 2021 11:53:22 +0000 Subject: [PATCH] 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. --- app/celery/service_callback_tasks.py | 2 +- app/config.py | 2 ++ scripts/paas_app_wrapper.sh | 2 +- tests/app/celery/test_service_callback_tasks.py | 2 +- tests/app/test_config.py | 3 ++- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py index 71431571d..ef7b0609c 100644 --- a/app/celery/service_callback_tasks.py +++ b/app/celery/service_callback_tasks.py @@ -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: try: - self.retry(queue=QueueNames.RETRY) + self.retry(queue=QueueNames.CALLBACKS_RETRY) except self.MaxRetriesExceededError: current_app.logger.warning( "Retry: {} has retried the max num of times for callback url {} and notification_id: {}".format( diff --git a/app/config.py b/app/config.py index 4bf384fe3..372334cab 100644 --- a/app/config.py +++ b/app/config.py @@ -27,6 +27,7 @@ class QueueNames(object): PROCESS_FTP = 'process-ftp-tasks' CREATE_LETTERS_PDF = 'create-letters-pdf-tasks' CALLBACKS = 'service-callbacks' + CALLBACKS_RETRY = 'service-callbacks-retry' LETTERS = 'letter-tasks' SMS_CALLBACKS = 'sms-callbacks' ANTIVIRUS = 'antivirus-tasks' @@ -50,6 +51,7 @@ class QueueNames(object): QueueNames.NOTIFY, QueueNames.CREATE_LETTERS_PDF, QueueNames.CALLBACKS, + QueueNames.CALLBACKS_RETRY, QueueNames.LETTERS, QueueNames.SMS_CALLBACKS, QueueNames.SAVE_API_EMAIL, diff --git a/scripts/paas_app_wrapper.sh b/scripts/paas_app_wrapper.sh index 989327cf0..7bb89d759 100755 --- a/scripts/paas_app_wrapper.sh +++ b/scripts/paas_app_wrapper.sh @@ -51,7 +51,7 @@ case $NOTIFY_APP_NAME in ;; delivery-worker-service-callbacks) 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) exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \ diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py index fa9305ca9..5450e5fce 100644 --- a/tests/app/celery/test_service_callback_tasks.py +++ b/tests/app/celery/test_service_callback_tasks.py @@ -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) 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", diff --git a/tests/app/test_config.py b/tests/app/test_config.py index efa6b8aff..dc5d79725 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -60,7 +60,7 @@ def test_load_config_if_cloudfoundry_not_available(reload_config): def test_queue_names_all_queues_correct(): # Need to ensure that all_queues() only returns queue names used in API queues = QueueNames.all_queues() - assert len(queues) == 17 + assert len(queues) == 18 assert set([ QueueNames.PRIORITY, QueueNames.PERIODIC, @@ -74,6 +74,7 @@ def test_queue_names_all_queues_correct(): QueueNames.NOTIFY, QueueNames.CREATE_LETTERS_PDF, QueueNames.CALLBACKS, + QueueNames.CALLBACKS_RETRY, QueueNames.LETTERS, QueueNames.SMS_CALLBACKS, QueueNames.SAVE_API_EMAIL,