diff --git a/app/celery/nightly_tasks.py b/app/celery/nightly_tasks.py index 92d401f46..54714922f 100644 --- a/app/celery/nightly_tasks.py +++ b/app/celery/nightly_tasks.py @@ -117,9 +117,12 @@ def timeout_notifications(): # so that we can cope with a high volume that need processing. We've changed # dao_timeout_notifications to return up to 100K notifications, so this task # will operate on up to 500K - normally we only get around 20K. + cutoff_time = datetime.utcnow() - timedelta( + seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + ) + for _ in range(0, 5): - notifications = \ - dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD')) + notifications = dao_timeout_notifications(cutoff_time) for notification in notifications: check_and_queue_callback_task(notification) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 15daca241..acfa286e3 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -489,11 +489,11 @@ def dao_delete_notifications_by_id(notification_id): ).delete(synchronize_session='fetch') -def dao_timeout_notifications(timeout_period_in_seconds): +def dao_timeout_notifications(cutoff_time): """ - Set email and SMS notifications (only) to "temporary-failure" status. + Set email and SMS notifications (only) to "temporary-failure" status + if they're still sending from before the specified cutoff_time. """ - timeout_start = datetime.utcnow() - timedelta(seconds=timeout_period_in_seconds) updated_at = datetime.utcnow() current_statuses = [NOTIFICATION_SENDING, NOTIFICATION_PENDING] new_status = NOTIFICATION_TEMPORARY_FAILURE @@ -503,13 +503,13 @@ def dao_timeout_notifications(timeout_period_in_seconds): # - we believe this is a OOM error but there are no logs. From # experimentation we've found we can safely process up to 100K. notifications = Notification.query.filter( - Notification.created_at < timeout_start, + Notification.created_at < cutoff_time, Notification.status.in_(current_statuses), Notification.notification_type.in_([SMS_TYPE, EMAIL_TYPE]) ).limit(100000).all() Notification.query.filter( - Notification.created_at < timeout_start, + Notification.created_at < cutoff_time, Notification.status.in_(current_statuses), Notification.notification_type.in_([SMS_TYPE, EMAIL_TYPE]), Notification.id.in_([n.id for n in notifications]), diff --git a/tests/app/celery/test_nightly_tasks.py b/tests/app/celery/test_nightly_tasks.py index 7a6167aa2..847a27ce9 100644 --- a/tests/app/celery/test_nightly_tasks.py +++ b/tests/app/celery/test_nightly_tasks.py @@ -161,17 +161,14 @@ def test_delete_letter_notifications_older_than_retention_calls_child_task(notif mocked.assert_called_once_with('letter') +@freeze_time("2021-12-13T10:00") def test_timeout_notifications(mocker, sample_notification): mock_update = mocker.patch('app.celery.nightly_tasks.check_and_queue_callback_task') mock_dao = mocker.patch('app.celery.nightly_tasks.dao_timeout_notifications') mock_dao.return_value = [sample_notification] timeout_notifications() - - mock_dao.assert_called_once_with( - current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') - ) - + mock_dao.assert_called_once_with(datetime.fromisoformat('2021-12-10T10:00')) mock_update.assert_called_once_with(sample_notification) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 8e5f495e2..e09bb26d8 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -671,7 +671,7 @@ def test_dao_timeout_notifications(sample_template): pending = create_notification(sample_template, status='pending') delivered = create_notification(sample_template, status='delivered') - temporary_failure_notifications = dao_timeout_notifications(1) + temporary_failure_notifications = dao_timeout_notifications(datetime.utcnow()) assert len(temporary_failure_notifications) == 2 assert Notification.query.get(created.id).status == 'created' @@ -687,7 +687,7 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_t pending = create_notification(sample_template, status='pending') delivered = create_notification(sample_template, status='delivered') - temporary_failure_notifications = dao_timeout_notifications(1) + temporary_failure_notifications = dao_timeout_notifications(datetime.utcnow()) assert len(temporary_failure_notifications) == 0 assert Notification.query.get(created.id).status == 'created' @@ -703,7 +703,7 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template) pending = create_notification(sample_letter_template, status='pending') delivered = create_notification(sample_letter_template, status='delivered') - temporary_failure_notifications = dao_timeout_notifications(1) + temporary_failure_notifications = dao_timeout_notifications(datetime.utcnow()) assert len(temporary_failure_notifications) == 0 assert Notification.query.get(created.id).status == 'created'