Merge pull request #129 from GSA/queue-investigation

SMS provider cleanup

app/celery/scheduled_tasks.py

@@ -32,13 +32,11 @@ from app.dao.jobs_dao import (
 from app.dao.notifications_dao import (
     dao_old_letters_with_created_status,
     dao_precompiled_letters_still_pending_virus_check,
-    is_delivery_slow_for_providers,
     letters_missing_from_sending_bucket,
     notifications_not_yet_sent,
 )
 from app.dao.provider_details_dao import (
     dao_adjust_provider_priority_back_to_resting_points,
-    dao_reduce_sms_provider_priority,
 )
 from app.dao.services_dao import (
     dao_find_services_sending_to_tv_numbers,
@@ -95,28 +93,6 @@ def delete_invitations():
         raise
 
 
-@notify_celery.task(name='switch-current-sms-provider-on-slow-delivery')
-def switch_current_sms_provider_on_slow_delivery():
-    """
-    Reduce provider's priority if at least 30% of notifications took more than four minutes to be delivered
-    in the last ten minutes. If both providers are slow, don't do anything. If we changed the providers in the
-    last ten minutes, then don't update them again either.
-    """
-    slow_delivery_notifications = is_delivery_slow_for_providers(
-        threshold=0.3,
-        created_at=datetime.utcnow() - timedelta(minutes=10),
-        delivery_time=timedelta(minutes=4),
-    )
-
-    # only adjust if some values are true and some are false - ie, don't adjust if all providers are fast or
-    # all providers are slow
-    if len(set(slow_delivery_notifications.values())) != 1:
-        for provider_name, is_slow in slow_delivery_notifications.items():
-            if is_slow:
-                current_app.logger.warning('Slow delivery notifications detected for provider {}'.format(provider_name))
-                dao_reduce_sms_provider_priority(provider_name, time_threshold=timedelta(minutes=10))
-
-
 @notify_celery.task(name='tend-providers-back-to-middle')
 def tend_providers_back_to_middle():
     dao_adjust_provider_priority_back_to_resting_points()

app/config.py

@@ -122,8 +122,9 @@ class Config(object):
     FIRETEXT_INTERNATIONAL_API_KEY = getenv("FIRETEXT_INTERNATIONAL_API_KEY", "placeholder")
     # these should always add up to 100%
     SMS_PROVIDER_RESTING_POINTS = {
-        'mmg': 50,
-        'firetext': 50
+        'sns': 100,
+        'mmg': 0,
+        'firetext': 0
     }
     FIRETEXT_INBOUND_SMS_AUTH = json.loads(getenv('FIRETEXT_INBOUND_SMS_AUTH', '[]'))
     MMG_INBOUND_SMS_AUTH = json.loads(getenv('MMG_INBOUND_SMS_AUTH', '[]'))
@@ -215,11 +216,6 @@ class Config(object):
                 'schedule': timedelta(minutes=66),
                 'options': {'queue': QueueNames.PERIODIC}
             },
-            'switch-current-sms-provider-on-slow-delivery': {
-                'task': 'switch-current-sms-provider-on-slow-delivery',
-                'schedule': crontab(),  # Every minute
-                'options': {'queue': QueueNames.PERIODIC}
-            },
             'check-job-status': {
                 'task': 'check-job-status',
                 'schedule': crontab(),

app/dao/notifications_dao.py

@@ -1,6 +1,4 @@
 from datetime import datetime, timedelta
-from itertools import groupby
-from operator import attrgetter
 
 from botocore.exceptions import ClientError
 from flask import current_app
@@ -16,14 +14,14 @@ from notifications_utils.timezones import (
     convert_local_timezone_to_utc,
     convert_utc_to_local_timezone,
 )
-from sqlalchemy import and_, asc, desc, func, or_, union
+from sqlalchemy import asc, desc, func, or_, union
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.sql import functions
 from sqlalchemy.sql.expression import case
 from werkzeug.datastructures import MultiDict
 
-from app import create_uuid, db, statsd_client
+from app import create_uuid, db
 from app.dao.dao_utils import autocommit
 from app.letters.utils import LetterPDFNotFound, find_letter_pdf_in_s3
 from app.models import (
@@ -32,7 +30,6 @@ from app.models import (
     KEY_TYPE_TEST,
     LETTER_TYPE,
     NOTIFICATION_CREATED,
-    NOTIFICATION_DELIVERED,
     NOTIFICATION_PENDING,
     NOTIFICATION_PENDING_VIRUS_CHECK,
     NOTIFICATION_PERMANENT_FAILURE,
@@ -44,7 +41,6 @@ from app.models import (
     FactNotificationStatus,
     Notification,
     NotificationHistory,
-    ProviderDetails,
 )
 from app.utils import (
     escape_special_characters,
@@ -453,61 +449,6 @@ def dao_timeout_notifications(cutoff_time, limit=100000):
     return notifications
 
 
-def is_delivery_slow_for_providers(
-        created_at,
-        threshold,
-        delivery_time,
-):
-    """
-    Returns a dict of providers and whether they are currently slow or not. eg:
-    {
-        'mmg': True,
-        'firetext': False
-    }
-    """
-    slow_notification_counts = db.session.query(
-        ProviderDetails.identifier,
-        case(
-            [(
-                Notification.status == NOTIFICATION_DELIVERED,
-                (Notification.updated_at - Notification.sent_at) >= delivery_time
-            )],
-            else_=(datetime.utcnow() - Notification.sent_at) >= delivery_time
-        ).label("slow"),
-        func.count().label('count')
-    ).select_from(
-        ProviderDetails
-    ).outerjoin(
-        Notification, and_(
-            Notification.notification_type == SMS_TYPE,
-            Notification.sent_by == ProviderDetails.identifier,
-            Notification.created_at >= created_at,
-            Notification.sent_at.isnot(None),
-            Notification.status.in_([NOTIFICATION_DELIVERED, NOTIFICATION_PENDING, NOTIFICATION_SENDING]),
-            Notification.key_type != KEY_TYPE_TEST
-        )
-    ).filter(
-        ProviderDetails.notification_type == 'sms',
-        ProviderDetails.active
-    ).order_by(
-        ProviderDetails.identifier
-    ).group_by(
-        ProviderDetails.identifier,
-        "slow"
-    )
-
-    slow_providers = {}
-    for provider, rows in groupby(slow_notification_counts, key=attrgetter('identifier')):
-        rows = list(rows)
-        total_notifications = sum(row.count for row in rows)
-        slow_notifications = sum(row.count for row in rows if row.slow)
-
-        slow_providers[provider] = (slow_notifications / total_notifications >= threshold)
-        statsd_client.gauge(f'slow-delivery.{provider}.ratio', slow_notifications / total_notifications)
-
-    return slow_providers
-
-
 @autocommit
 def dao_update_notifications_by_reference(references, update_dict):
     updated_count = Notification.query.filter(