order the insert/update by created_at

you should always order by when doing a limit/offset, to guarantee that
the second time you run that query, the order hasn't changed and you
aren't just repeating the task with an overlap of notifications.
Luckily, in this case we haven't lost any data, because:

* we have an on conflict do update statement so when we returned
  duplicate rows it would just do an update
* when we delete, we cross-reference with the notification history so if
  a row always got missed, we won't delete it.

This resulted in, for example, govuk email still having a handful of
notifications in the table from 9th despite the task running succesfully
every day until the 18th of march.

order by created_at ascending so that we start with oldest notifications
first, in case it's interrupted part way through.
This commit is contained in:
Leo Hemsted
2020-03-20 19:11:24 +00:00
parent dc5b56ff78
commit 8ea09be1ff

View File

@@ -420,6 +420,8 @@ def insert_update_notification_history(notification_type, date_to_delete_from, s
Notification.service_id == service_id,
Notification.created_at < date_to_delete_from,
Notification.key_type != KEY_TYPE_TEST
).order_by(
Notification.created_at
)
notifications_count = notification_query.count()