From a3088157935d13f7698a43a838ea6875c23556ef Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 16 Mar 2018 14:12:59 +0000 Subject: [PATCH] We had an exception today caused by a slow running query. This PR will optimize this query to use a more efficient index. - Add notification_type to the dao_get_last_template_usage to optimize the query. - Tested and analyzed query on production database with very significant results. Before: QUERY PLAN ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Limit (cost=0.43..1711.35 rows=1 width=935) (actual time=21186.053..21186.053 rows=0 loops=1) -> Index Scan Backward using ix_notifications_created_at on notifications (cost=0.43..4607493.80 rows=2693 width=935) (actual time=21186.052..21186.052 rows=0 loops=1) Filter: (((key_type)::text <> 'test'::text) AND (template_id = 'xxxxxx'::uuid)) Rows Removed by Filter: 8244071 Planning time: 0.112 ms Execution time: 21186.082 ms After: QUERY PLAN --------------------------------------------------------------------------------------------------------------------- Limit (cost=5323.10..5323.10 rows=1 width=935) -> Sort (cost=5323.10..5323.74 rows=258 width=935) Sort Key: created_at DESC -> Index Scan using ix_notifications_template_id on notifications (cost=0.56..5321.81 rows=258 width=935) Index Cond: (template_id = 'xxxxx'::uuid) Filter: (((key_type)::text <> 'test'::text) AND (notification_type = 'sms'::notification_type)) Planning time: 1.102 ms Execution time: 0.584 ms --- app/dao/notifications_dao.py | 5 +++-- app/template_statistics/rest.py | 2 +- .../notification_dao/test_notification_dao.py | 18 +++++++++--------- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index d55d546db..37c7dddfb 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -96,10 +96,11 @@ def dao_get_template_usage(service_id, limit_days=None): @statsd(namespace="dao") -def dao_get_last_template_usage(template_id): +def dao_get_last_template_usage(template_id, template_type): return Notification.query.filter( Notification.template_id == template_id, - Notification.key_type != KEY_TYPE_TEST + Notification.key_type != KEY_TYPE_TEST, + Notification.notification_type == template_type ).order_by( desc(Notification.created_at) ).first() diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index b7198409e..950b5ecd6 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -63,7 +63,7 @@ def get_template_statistics_for_template_id(service_id, template_id): raise InvalidRequest(errors, status_code=404) data = None - notification = dao_get_last_template_usage(template_id) + notification = dao_get_last_template_usage(template_id, template.template_type) if notification: data = notification_with_template_schema.dump(notification).data diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 3ebbb2a0d..b6d7da722 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -86,7 +86,7 @@ def test_should_be_able_to_get_template_usage_history(notify_db, notify_db_sessi with freeze_time('2000-01-01 12:00:00'): sms = create_sample_template(notify_db, notify_db_session) notification = sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) - results = dao_get_last_template_usage(sms.id) + results = dao_get_last_template_usage(sms.id, 'sms') assert results.template.name == 'Template Name' assert results.template.template_type == 'sms' assert results.created_at == datetime(year=2000, month=1, day=1, hour=12, minute=0, second=0) @@ -98,14 +98,14 @@ def test_should_be_able_to_get_all_template_usage_history_order_by_notification_ notify_db, notify_db_session, sample_service): - sms = create_sample_template(notify_db, notify_db_session) + email = create_sample_template(notify_db, notify_db_session, template_type='email') - sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) - sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) - sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) - most_recent = sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, service=sample_service, template=email) + most_recent = sample_notification(notify_db, notify_db_session, service=sample_service, template=email) - results = dao_get_last_template_usage(sms.id) + results = dao_get_last_template_usage(email.id, 'email') assert results.id == most_recent.id @@ -135,7 +135,7 @@ def test_template_usage_should_ignore_test_keys( api_key=sample_test_api_key, key_type=KEY_TYPE_TEST) - results = dao_get_last_template_usage(sms.id) + results = dao_get_last_template_usage(sms.id, 'sms') assert results.id == team_key.id @@ -144,7 +144,7 @@ def test_should_be_able_to_get_no_template_usage_history_if_no_notifications_usi notify_db_session): sms = create_sample_template(notify_db, notify_db_session) - results = dao_get_last_template_usage(sms.id) + results = dao_get_last_template_usage(sms.id, 'sms') assert not results