Merge pull request #8 from Flagsmith/refac/cleanup-tasks

refactor(clean_up_old_tasks): remove exists query

task_processor/tasks.py

@@ -37,19 +37,16 @@ def clean_up_old_tasks():
         query = query | Q(num_failures__gte=3)
     query = Q(scheduled_for__lt=delete_before) & query
 
-    # We define the query in the loop and in the delete query to avoid having
-    # an infinite loop since we need to verify if there are records in the qs
-    # on each iteration of the loop. Defining the queryset outside of the
-    # loop condition leads to queryset.exists() always returning true resulting
-    # in an infinite loop.
     # TODO: validate if deleting in batches is more / less impactful on the DB
-    while Task.objects.filter(query).exists():
+    while True:
         # delete in batches of settings.TASK_DELETE_BATCH_SIZE
-        Task.objects.filter(
+        num_tasks_deleted, _ = Task.objects.filter(
             pk__in=Task.objects.filter(query).values_list("id", flat=True)[
                 0 : settings.TASK_DELETE_BATCH_SIZE  # noqa:E203
             ]
         ).delete()
+        if num_tasks_deleted == 0:
+            break
 
 
 @register_recurring_task(

tests/settings.py

@@ -20,3 +20,5 @@
 TASK_DELETE_BATCH_SIZE = 2000
 
 DATABASES = {"default": dj_database_url.parse(env("DATABASE_URL"))}
+
+USE_TZ = True

tests/unit/task_processor/test_unit_task_processor_tasks.py

@@ -68,15 +68,14 @@ def test_clean_up_old_tasks(settings, django_assert_num_queries, db):
     )
 
     # When
-    with django_assert_num_queries(9):
+    with django_assert_num_queries(7):
         # We expect 9 queries to be run here since we have set the delete batch size to 1 and there are 2
-        # tasks we expect it to delete. Therefore, we have 2 loops, each consisting of 4 queries:
-        #  1. Check if any tasks matching the query exist
-        #  2. Grab the ids of any matching tasks
-        #  3. Delete all TaskRun objects for those task_id values
-        #  4. Delete all Task objects for those ids
+        # tasks we expect it to delete. Therefore, we have 2 loops, each consisting of 3 queries:
+        #  1. Grab the ids of any matching tasks
+        #  2. Delete all TaskRun objects for those task_id values
+        #  3. Delete all Task objects for those ids
         #
-        # The final (9th) query is checking if any tasks exist again (which returns false).
+        # The final (7th) query is checking if any tasks match the delete filter (which returns false).
         clean_up_old_tasks()
 
     # Then