From 51c6d57a86dd1b13fe74a8127099313a59b8c4af Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 17 Jun 2016 16:32:56 +0100 Subject: [PATCH] Changed the delay period. Now waits 10 seconds, 1 minute, 5 minutes, 1 hour and 4 hours. Total elapsed wait is max 5 hours 6 minutes and 10 seconds. Changed visibility window of SQS to be 4 hours 10 seconds, longer the max retry period. --- app/celery/provider_tasks.py | 26 +++++++++++++-------- config.py | 2 +- tests/app/celery/test_provider_tasks.py | 30 ++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 57d01889c..c599313ab 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -24,19 +24,27 @@ from notifications_utils.template import ( ) -def retry_iteration_to_delay(retry): +def retry_iteration_to_delay(retry=0): """ Given current retry calculate some delay before retrying - Delay calculated as (1 + retry number) squared * 60 - 0: 60 seconds (1 minute) - 1: 240 seconds (4 minutes) - 2: 540 seconds (9 minutes) - 3: 960 seconds (16 minutes) - 4: 1500 seconds (25 minutes) + 0: 10 seconds + 1: 60 seconds (1 minutes) + 2: 300 seconds (5 minutes) + 3: 3600 seconds (60 minutes) + 4: 14400 seconds (4 hours) :param retry (zero indexed): - :return length to retry in seconds: + :return length to retry in seconds, default 10 seconds """ - return ((retry + 1) ** 2) * 60 + + delays = { + 0: 10, + 1: 60, + 2: 300, + 3: 3600, + 4: 14400 + } + + return delays.get(retry, 10) @notify_celery.task(bind=True, name="send-sms-to-provider", max_retries=5, default_retry_delay=5) diff --git a/config.py b/config.py index 6b7f56eae..6973fe36f 100644 --- a/config.py +++ b/config.py @@ -38,7 +38,7 @@ class Config(object): BROKER_TRANSPORT_OPTIONS = { 'region': 'eu-west-1', 'polling_interval': 1, # 1 second - 'visibility_timeout': 60, # 60 seconds + 'visibility_timeout': 14410, # 4 hours 10 seconds. 10 seconds longer than max retry 'queue_name_prefix': os.environ['NOTIFICATION_QUEUE_PREFIX'] + '-' } CELERY_ENABLE_UTC = True, diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 44d2440fa..f4a899ffd 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -20,6 +20,34 @@ from tests.app.conftest import ( ) +def test_should_by_10_second_delay_as_default(): + assert provider_tasks.retry_iteration_to_delay() == 10 + + +def test_should_by_10_second_delay_on_unmapped_retry_iteration(): + assert provider_tasks.retry_iteration_to_delay(99) == 10 + + +def test_should_by_10_second_delay_on_retry_one(): + assert provider_tasks.retry_iteration_to_delay(0) == 10 + + +def test_should_by_1_minute_delay_on_retry_two(): + assert provider_tasks.retry_iteration_to_delay(1) == 60 + + +def test_should_by_5_minute_delay_on_retry_two(): + assert provider_tasks.retry_iteration_to_delay(2) == 300 + + +def test_should_by_60_minute_delay_on_retry_two(): + assert provider_tasks.retry_iteration_to_delay(3) == 3600 + + +def test_should_by_240_minute_delay_on_retry_two(): + assert provider_tasks.retry_iteration_to_delay(4) == 14400 + + def test_should_return_highest_priority_active_provider(notify_db, notify_db_session): providers = provider_details_dao.get_provider_details_by_notification_type('sms') first = providers[0] @@ -324,7 +352,7 @@ def test_should_go_into_technical_error_if_exceeds_retries( "encrypted-in-reality" ) - provider_tasks.send_sms_to_provider.retry.assert_called_with(queue='retry', countdown=60) + provider_tasks.send_sms_to_provider.retry.assert_called_with(queue='retry', countdown=10) assert statsd_client.incr.assert_not_called assert statsd_client.timing.assert_not_called