From 21edf7bfddd2aec44d1503555e29aed7b19855e4 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 22 Feb 2021 15:42:29 +0000 Subject: [PATCH] Persist the processing time statistics to the database. The performance platform is going away soon. The only stat that we do not have in our database is the processing time. Let me clarify the only statistic we don't have in our database that we can query efficiently is the processing time. Any queries on notification_history are too inefficient to use on a web page. Processing time = the total number of normal/team emails and text messages plus the number of messages that have gone from created to sending within 10 seconds per whole day. We can then easily calculate the percentage of messages that were marked as sending under 10 seconds. --- app/commands.py | 2 +- app/dao/fact_processing_time_dao.py | 29 +++++++++++++++ app/dao/notifications_dao.py | 4 +-- app/models.py | 8 +++++ app/performance_platform/processing_time.py | 11 ++++-- .../versions/0347_add_ft_processing_time.py | 28 +++++++++++++++ .../test_performance_platform_commands.py | 6 ++-- .../app/dao/test_fact_processing_time_dao.py | 36 +++++++++++++++++++ .../test_processing_time.py | 6 ++++ 9 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 app/dao/fact_processing_time_dao.py create mode 100644 migrations/versions/0347_add_ft_processing_time.py create mode 100644 tests/app/dao/test_fact_processing_time_dao.py diff --git a/app/commands.py b/app/commands.py index cc34d4f80..e04e52a8b 100644 --- a/app/commands.py +++ b/app/commands.py @@ -269,7 +269,7 @@ def backfill_processing_time(start_date, end_date): process_start_date.isoformat(), process_end_date.isoformat() )) - send_processing_time_for_start_and_end(process_start_date, process_end_date) + send_processing_time_for_start_and_end(process_start_date, process_end_date, process_date) @notify_command(name='populate-annual-billing') diff --git a/app/dao/fact_processing_time_dao.py b/app/dao/fact_processing_time_dao.py new file mode 100644 index 000000000..3de9df8e9 --- /dev/null +++ b/app/dao/fact_processing_time_dao.py @@ -0,0 +1,29 @@ +from sqlalchemy.dialects.postgresql import insert + +from app import db +from app.dao.dao_utils import transactional +from app.models import FactProcessingTime + + +@transactional +def insert_update_processing_time(processing_time): + ''' + This uses the Postgres upsert to avoid race conditions when two threads try and insert + at the same row. The excluded object refers to values that we tried to insert but were + rejected. + http://docs.sqlalchemy.org/en/latest/dialects/postgresql.html#insert-on-conflict-upsert + ''' + table = FactProcessingTime.__table__ + stmt = insert(table).values( + bst_date=processing_time.bst_date, + messages_total=processing_time.messages_total, + messages_within_10_secs=processing_time.messages_within_10_secs + ) + stmt = stmt.on_conflict_do_update( + index_elements=[table.c.bst_date], + set_={ + 'messages_total': stmt.excluded.messages_total, + 'messages_within_10_secs': stmt.excluded.messages_within_10_secs + } + ) + db.session.connection().execute(stmt) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index bc18a8f43..599468810 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -683,9 +683,9 @@ def dao_get_notifications_by_references(references): def dao_get_total_notifications_sent_per_day_for_performance_platform(start_date, end_date): """ SELECT - count(notification_history), + count(notifications), coalesce(sum(CASE WHEN sent_at - created_at <= interval '10 seconds' THEN 1 ELSE 0 END), 0) - FROM notification_history + FROM notifications WHERE created_at > 'START DATE' AND created_at < 'END DATE' AND diff --git a/app/models.py b/app/models.py index b2f55eec3..9cf73e4c7 100644 --- a/app/models.py +++ b/app/models.py @@ -2068,6 +2068,14 @@ class FactNotificationStatus(db.Model): updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) +class FactProcessingTime(db.Model): + __tablename__ = "ft_processing_time" + + bst_date = db.Column(db.Date, index=True, primary_key=True, nullable=False) + messages_total = db.Column(db.Integer(), nullable=False) + messages_within_10_secs = db.Column(db.Integer(), nullable=False) + + class Complaint(db.Model): __tablename__ = 'complaints' diff --git a/app/performance_platform/processing_time.py b/app/performance_platform/processing_time.py index 756c08bc1..d93764a48 100644 --- a/app/performance_platform/processing_time.py +++ b/app/performance_platform/processing_time.py @@ -2,6 +2,8 @@ from datetime import timedelta from flask import current_app +from app.dao.fact_processing_time_dao import insert_update_processing_time +from app.models import FactProcessingTime from app.utils import get_london_midnight_in_utc from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_performance_platform from app import performance_platform_client @@ -11,10 +13,10 @@ def send_processing_time_to_performance_platform(bst_date): start_time = get_london_midnight_in_utc(bst_date) end_time = get_london_midnight_in_utc(bst_date + timedelta(days=1)) - send_processing_time_for_start_and_end(start_time, end_time) + send_processing_time_for_start_and_end(start_time, end_time, bst_date) -def send_processing_time_for_start_and_end(start_time, end_time): +def send_processing_time_for_start_and_end(start_time, end_time, bst_date): result = dao_get_total_notifications_sent_per_day_for_performance_platform(start_time, end_time) current_app.logger.info( @@ -25,6 +27,11 @@ def send_processing_time_for_start_and_end(start_time, end_time): send_processing_time_data(start_time, 'messages-total', result.messages_total) send_processing_time_data(start_time, 'messages-within-10-secs', result.messages_within_10_secs) + insert_update_processing_time(FactProcessingTime( + bst_date=bst_date, + messages_total=result.messages_total, + messages_within_10_secs=result.messages_within_10_secs) + ) def send_processing_time_data(start_time, status, count): diff --git a/migrations/versions/0347_add_ft_processing_time.py b/migrations/versions/0347_add_ft_processing_time.py new file mode 100644 index 000000000..0e4905a40 --- /dev/null +++ b/migrations/versions/0347_add_ft_processing_time.py @@ -0,0 +1,28 @@ +""" + +Revision ID: 0347_add_ft_processing_time +Revises: 0346_notify_number_sms_sender +Create Date: 2021-02-22 14:05:24.775338 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0347_add_ft_processing_time' +down_revision = '0346_notify_number_sms_sender' + + +def upgrade(): + op.create_table('ft_processing_time', + sa.Column('bst_date', sa.Date(), nullable=False), + sa.Column('messages_total', sa.Integer(), nullable=False), + sa.Column('messages_within_10_secs', sa.Integer(), nullable=False), + sa.PrimaryKeyConstraint('bst_date') + ) + op.create_index(op.f('ix_ft_processing_time_bst_date'), 'ft_processing_time', ['bst_date'], unique=False) + + +def downgrade(): + op.drop_index(op.f('ix_ft_processing_time_bst_date'), table_name='ft_processing_time') + op.drop_table('ft_processing_time') diff --git a/tests/app/commands/test_performance_platform_commands.py b/tests/app/commands/test_performance_platform_commands.py index e7b8ac360..8e5186b9e 100644 --- a/tests/app/commands/test_performance_platform_commands.py +++ b/tests/app/commands/test_performance_platform_commands.py @@ -11,9 +11,9 @@ def test_backfill_processing_time_works_for_correct_dates(mocker, notify_api): backfill_processing_time.callback.__wrapped__(datetime(2017, 8, 1), datetime(2017, 8, 3)) assert send_mock.call_count == 3 - send_mock.assert_any_call(datetime(2017, 7, 31, 23, 0), datetime(2017, 8, 1, 23, 0)) - send_mock.assert_any_call(datetime(2017, 8, 1, 23, 0), datetime(2017, 8, 2, 23, 0)) - send_mock.assert_any_call(datetime(2017, 8, 2, 23, 0), datetime(2017, 8, 3, 23, 0)) + send_mock.assert_any_call(datetime(2017, 7, 31, 23, 0), datetime(2017, 8, 1, 23, 0), datetime(2017, 8, 2, 0, 0)) + send_mock.assert_any_call(datetime(2017, 8, 1, 23, 0), datetime(2017, 8, 2, 23, 0), datetime(2017, 8, 3, 0, 0)) + send_mock.assert_any_call(datetime(2017, 8, 2, 23, 0), datetime(2017, 8, 3, 23, 0), datetime(2017, 8, 4, 0, 0)) def test_backfill_totals_works_for_correct_dates(mocker, notify_api): diff --git a/tests/app/dao/test_fact_processing_time_dao.py b/tests/app/dao/test_fact_processing_time_dao.py new file mode 100644 index 000000000..0fa2d5d61 --- /dev/null +++ b/tests/app/dao/test_fact_processing_time_dao.py @@ -0,0 +1,36 @@ +from datetime import datetime + +from app.dao import fact_processing_time_dao +from app.models import FactProcessingTime + + +def test_insert_update_processing_time(notify_db_session): + data = FactProcessingTime( + bst_date=datetime(2021, 2, 22).date(), + messages_total=3, + messages_within_10_secs=2 + ) + + fact_processing_time_dao.insert_update_processing_time(data) + + result = FactProcessingTime.query.all() + + assert len(result) == 1 + assert result[0].bst_date == datetime(2021, 2, 22).date() + assert result[0].messages_total == 3 + assert result[0].messages_within_10_secs == 2 + + data = FactProcessingTime( + bst_date=datetime(2021, 2, 22).date(), + messages_total=4, + messages_within_10_secs=3 + ) + + fact_processing_time_dao.insert_update_processing_time(data) + + result = FactProcessingTime.query.all() + + assert len(result) == 1 + assert result[0].bst_date == datetime(2021, 2, 22).date() + assert result[0].messages_total == 4 + assert result[0].messages_within_10_secs == 3 diff --git a/tests/app/performance_platform/test_processing_time.py b/tests/app/performance_platform/test_processing_time.py index 010c50097..f9f0ac9b4 100644 --- a/tests/app/performance_platform/test_processing_time.py +++ b/tests/app/performance_platform/test_processing_time.py @@ -2,6 +2,7 @@ from datetime import datetime, timedelta, date from freezegun import freeze_time +from app.models import FactProcessingTime from tests.app.db import create_notification from app.performance_platform.processing_time import ( send_processing_time_to_performance_platform, @@ -23,6 +24,11 @@ def test_send_processing_time_to_performance_platform_generates_correct_calls(mo send_mock.assert_any_call(datetime(2016, 10, 16, 23, 0), 'messages-total', 2) send_mock.assert_any_call(datetime(2016, 10, 16, 23, 0), 'messages-within-10-secs', 1) + persisted_to_db = FactProcessingTime.query.all() + assert len(persisted_to_db) == 1 + assert persisted_to_db[0].bst_date == date(2016, 10, 17) + assert persisted_to_db[0].messages_total == 2 + assert persisted_to_db[0].messages_within_10_secs == 1 def test_send_processing_time_to_performance_platform_creates_correct_call_to_perf_platform(mocker):