From 085e3a8435bc7385c16d5cb1a6344db333482481 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Tue, 7 Apr 2020 16:41:33 +0100 Subject: [PATCH] Make deleting of notifications to be sequential Based on this https://github.com/alphagov/notifications-api/pull/2788 where some concerns were raised. This should be a quicker fix to get our the deletions to run sequentially for all the notification types. Note, email is first as most important as makes up the larger numbers (we wouldn't want it to start with SMS, fail half way for a reason that only affects SMS and for that to affect the email deletion). We hope that by running sequentially we will reduce conflicts writing to the same index and this will speed up the total time it takes to finish deleting all notification types older than their retention time. There is a risk that whilst quicker per job, as they now run sequentially rather than potentially overlapping, they will take longer overall. We will need to monitor to see. --- app/celery/nightly_tasks.py | 8 ++++++++ app/config.py | 15 ++------------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/app/celery/nightly_tasks.py b/app/celery/nightly_tasks.py index 080445df1..ca48955f0 100644 --- a/app/celery/nightly_tasks.py +++ b/app/celery/nightly_tasks.py @@ -62,6 +62,14 @@ def _remove_csv_files(job_types): current_app.logger.info("Job ID {} has been removed from s3.".format(job.id)) +@notify_celery.task(name="delete-notifications-older-than-retention") +@statsd(namespace="tasks") +def delete_notifications_older_than_retention(): + delete_email_notifications_older_than_retention() + delete_sms_notifications_older_than_retention() + delete_letter_notifications_older_than_retention() + + @notify_celery.task(name="delete-sms-notifications") @cronitor("delete-sms-notifications") @statsd(namespace="tasks") diff --git a/app/config.py b/app/config.py index 7dba73014..f31bf49e6 100644 --- a/app/config.py +++ b/app/config.py @@ -238,27 +238,16 @@ class Config(object): 'schedule': crontab(hour=0, minute=30), # after 'timeout-sending-notifications' 'options': {'queue': QueueNames.REPORTING} }, - 'delete-sms-notifications': { - 'task': 'delete-sms-notifications', + 'delete-notifications-older-than-retention': { + 'task': 'delete-notifications-older-than-retention', 'schedule': crontab(hour=4, minute=15), # after 'create-nightly-notification-status' 'options': {'queue': QueueNames.PERIODIC} }, - 'delete-email-notifications': { - 'task': 'delete-email-notifications', - 'schedule': crontab(hour=4, minute=30), # after 'create-nightly-notification-status' - 'options': {'queue': QueueNames.PERIODIC} - }, - 'delete-letter-notifications': { - 'task': 'delete-letter-notifications', - 'schedule': crontab(hour=4, minute=45), # after 'create-nightly-notification-status' - 'options': {'queue': QueueNames.PERIODIC} - }, 'delete-inbound-sms': { 'task': 'delete-inbound-sms', 'schedule': crontab(hour=1, minute=40), 'options': {'queue': QueueNames.PERIODIC} }, - 'send-daily-performance-platform-stats': { 'task': 'send-daily-performance-platform-stats', 'schedule': crontab(hour=2, minute=0),