From 787d0da7dbcee51f04542e1e8c6449a1f4410eb3 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 4 Apr 2017 11:23:54 +0100 Subject: [PATCH 1/5] Broke the periodic queue out from the standard worker. - now has own worker with worker count of 2 - this is too ensure that any issues with scheduled jobs do not affect core applications --- app/config.py | 13 +++++++------ manifest-delivery-base.yml | 5 +++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/config.py b/app/config.py index 619532d3d..5b3e7e309 100644 --- a/app/config.py +++ b/app/config.py @@ -119,17 +119,17 @@ class Config(object): }, 'delete-failed-notifications': { 'task': 'delete-failed-notifications', - 'schedule': crontab(minute=15, hour='0,1,2'), + 'schedule': crontab(minute=0, hour=0), 'options': {'queue': 'periodic'} }, 'delete-successful-notifications': { 'task': 'delete-successful-notifications', - 'schedule': crontab(minute=0, hour='0,1,2'), + 'schedule': crontab(minute=0, hour=1), 'options': {'queue': 'periodic'} }, 'send-daily-performance-platform-stats': { 'task': 'send-daily-performance-platform-stats', - 'schedule': crontab(minute=0, hour=4), # 04:00 + 'schedule': crontab(minute=0, hour=2), 'options': {'queue': 'periodic'} }, 'switch-current-sms-provider-on-slow-delivery': { @@ -139,17 +139,16 @@ class Config(object): }, 'timeout-sending-notifications': { 'task': 'timeout-sending-notifications', - 'schedule': crontab(minute=30, hour='0,1,2'), + 'schedule': crontab(minute=0, hour=3), 'options': {'queue': 'periodic'} }, 'remove_csv_files': { 'task': 'remove_csv_files', - 'schedule': crontab(minute=45, hour='0,1,2'), + 'schedule': crontab(minute=0, hour=4), 'options': {'queue': 'periodic'} } } CELERY_QUEUES = [ - Queue('periodic', Exchange('default'), routing_key='periodic'), Queue('process-job', Exchange('default'), routing_key='process-job'), Queue('retry', Exchange('default'), routing_key='retry'), Queue('notify', Exchange('default'), routing_key='notify') @@ -191,6 +190,7 @@ class Development(Config): SQLALCHEMY_ECHO = False CELERY_QUEUES = Config.CELERY_QUEUES + [ Queue('db-sms', Exchange('default'), routing_key='db-sms'), + Queue('periodic', Exchange('default'), routing_key='periodic'), Queue('db-email', Exchange('default'), routing_key='db-email'), Queue('db-letter', Exchange('default'), routing_key='db-letter'), Queue('send-sms', Exchange('default'), routing_key='send-sms'), @@ -210,6 +210,7 @@ class Test(Config): STATSD_HOST = "localhost" STATSD_PORT = 1000 CELERY_QUEUES = Config.CELERY_QUEUES + [ + Queue('periodic', Exchange('default'), routing_key='periodic'), Queue('db-sms', Exchange('default'), routing_key='db-sms'), Queue('db-email', Exchange('default'), routing_key='db-email'), Queue('db-letter', Exchange('default'), routing_key='db-letter'), diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index b60916916..06f53a178 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -35,6 +35,11 @@ applications: env: NOTIFY_APP_NAME: delivery-worker-sender + - name: notify-delivery-worker-periodic + command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=2 -Q periodic + env: + NOTIFY_APP_NAME: delivery-worker + - name: notify-delivery-worker command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 env: From ce09765dad324fe249357f10e5492b60554247e6 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 4 Apr 2017 13:42:21 +0100 Subject: [PATCH 2/5] Turned down the Memory usage on prod. This was bumped to 2G to try and offset memory usage which caused the delivery worker to die. We are now spinning this periodic worker off into it's own worker, so it's not bundled with the app. this means we can set it's memory usage specifically so we don't need to bump all the workers. --- manifest-delivery-production.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifest-delivery-production.yml b/manifest-delivery-production.yml index f6f27d21a..53c8d2f12 100644 --- a/manifest-delivery-production.yml +++ b/manifest-delivery-production.yml @@ -3,4 +3,4 @@ inherit: manifest-delivery-base.yml instances: 2 -memory: 2G +memory: 1G From 43bf973b1d323663cbdc679f9b1c78fc82ec5c0a Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 4 Apr 2017 13:43:28 +0100 Subject: [PATCH 3/5] A few changes to the celery workers 1) Beat worker now a single instance with 128M memory 2) New worker for periodic tasks. Single worker, 2G ram 3) new worker for priority queues. Standard instance and memory settings. --- manifest-delivery-base.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index 06f53a178..2b85b9d15 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -17,6 +17,8 @@ memory: 1G applications: - name: notify-delivery-celery-beat command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery beat --loglevel=INFO + instances: 1 + memory: 128M env: NOTIFY_APP_NAME: delivery-celery-beat @@ -37,8 +39,15 @@ applications: - name: notify-delivery-worker-periodic command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=2 -Q periodic + instances: 1 + memory: 2G env: - NOTIFY_APP_NAME: delivery-worker + NOTIFY_APP_NAME: delivery-worker-periodic + + - name: notify-delivery-worker-priority + command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=5 -Q priority + env: + NOTIFY_APP_NAME: delivery-worker-priority - name: notify-delivery-worker command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 From ac445114c767755efb3812aeeaf50ad0ac77151a Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 4 Apr 2017 13:44:11 +0100 Subject: [PATCH 4/5] When using a priority template put the notification in the priority queue. - there is now a dedicated worker to handle priority queues. We don't now bundle this in the default worker. --- app/notifications/rest.py | 2 +- app/v2/notifications/post_notifications.py | 2 +- tests/app/notifications/rest/test_send_notification.py | 2 +- tests/app/v2/notifications/test_post_notifications.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 4aa6283c1..57e1efeed 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -131,7 +131,7 @@ def send_notification(notification_type): key_type=api_user.key_type, simulated=simulated) if not simulated: - queue_name = 'notify' if template.process_type == PRIORITY else None + queue_name = 'priority' if template.process_type == PRIORITY else None send_notification_to_queue(notification=notification_model, research_mode=service.research_mode, queue=queue_name) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 6b0f59ec1..c3cbfdabf 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -50,7 +50,7 @@ def post_notification(notification_type): reference=form.get('reference', None), simulated=simulated) if not simulated: - queue_name = 'notify' if template.process_type == PRIORITY else None + queue_name = 'priority' if template.process_type == PRIORITY else None send_notification_to_queue(notification=notification, research_mode=service.research_mode, queue=queue_name) else: current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index cde289b58..7c90f0c99 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -1022,4 +1022,4 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori notification_id = response_data['notification']['id'] assert response.status_code == 201 - mocked.assert_called_once_with([notification_id], queue='notify') + mocked.assert_called_once_with([notification_id], queue='priority') diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 8ad0204e8..bf15dd8e3 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -223,4 +223,4 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori notification_id = json.loads(response.data)['id'] assert response.status_code == 201 - mocked.assert_called_once_with([notification_id], queue='notify') + mocked.assert_called_once_with([notification_id], queue='priority') From b08ac6eb6314b09291cfaafc45a48241a4ae4be7 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 4 Apr 2017 13:53:01 +0100 Subject: [PATCH 5/5] Ensure priority queue read by development machines. It's now a default queue in the config. --- app/config.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/config.py b/app/config.py index 5b3e7e309..5dd33557e 100644 --- a/app/config.py +++ b/app/config.py @@ -190,6 +190,7 @@ class Development(Config): SQLALCHEMY_ECHO = False CELERY_QUEUES = Config.CELERY_QUEUES + [ Queue('db-sms', Exchange('default'), routing_key='db-sms'), + Queue('priority', Exchange('default'), routing_key='priority'), Queue('periodic', Exchange('default'), routing_key='periodic'), Queue('db-email', Exchange('default'), routing_key='db-email'), Queue('db-letter', Exchange('default'), routing_key='db-letter'), @@ -211,6 +212,7 @@ class Test(Config): STATSD_PORT = 1000 CELERY_QUEUES = Config.CELERY_QUEUES + [ Queue('periodic', Exchange('default'), routing_key='periodic'), + Queue('priority', Exchange('default'), routing_key='priority'), Queue('db-sms', Exchange('default'), routing_key='db-sms'), Queue('db-email', Exchange('default'), routing_key='db-email'), Queue('db-letter', Exchange('default'), routing_key='db-letter'),