From a66a8419b954281bf40f5e2ef2e3ff8920f9b66e Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Fri, 19 Jun 2020 10:54:02 -0700 Subject: [PATCH] bankruptcy: Remove broken push notifications loop. The loop I added here in 5b49839b08b2223c0454995a0808ea9df2ecb1b3 was ill-conceived. The critical issue was that despite its name, do_clear_mobile_push_notifications_for_ids does not immediately clear push notifications (Except in our test suite, where `send_event` immediately calls into the queue worker code!). Instead, it queues work to clear those push notifications. Which means that the first user to declare bankruptcy with a large number of unreads will fill the queue, and then this will just be an infinite loop adding more work to the queue. --- zerver/lib/actions.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 6f54851c12..4da809a51c 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -4078,17 +4078,12 @@ def do_mark_all_as_read(user_profile: UserProfile, client: Client) -> int: # First, we clear mobile push notifications. This is safer in the # event that the below logic times out and we're killed. - while True: - all_push_message_ids = UserMessage.objects.filter( - user_profile=user_profile, - ).extra( - where=[UserMessage.where_active_push_notification()], - ).values_list("message_id", flat=True)[0:10000] - - if len(all_push_message_ids) == 0: - break - - do_clear_mobile_push_notifications_for_ids(user_profile, all_push_message_ids) + all_push_message_ids = UserMessage.objects.filter( + user_profile=user_profile, + ).extra( + where=[UserMessage.where_active_push_notification()], + ).values_list("message_id", flat=True)[0:10000] + do_clear_mobile_push_notifications_for_ids(user_profile, all_push_message_ids) msgs = UserMessage.objects.filter( user_profile=user_profile,