frontend: speedup the removal of temporary projects

Fix #2489

I tried this on today's database dump and it took 46m24.731s to
finish and my laptop was fine. I think the crash was prevented by
not committing everything at once.

frontend/coprs_frontend/commands/clean_expired_projects.py

@@ -1,6 +1,6 @@
 import click
+from coprs import db
 from . import deprioritize_actions
-from coprs import db_session_scope
 from coprs.logic.complex_logic import ComplexLogic
 
 
@@ -11,6 +11,5 @@ def clean_expired_projects():
     Clean all the expired temporary projects.  This command is meant to be
     executed by cron.
     """
-
-    with db_session_scope():
-        ComplexLogic.delete_expired_projects()
+    while ComplexLogic.delete_expired_projects(limit=100):
+        db.session.commit()

frontend/coprs_frontend/coprs/logic/builds_logic.py

@@ -1279,12 +1279,14 @@ def delete_build(cls, user, build, send_delete_action=True):
         db.session.delete(build)
 
     @classmethod
-    def delete_builds(cls, user, build_ids):
+    def delete_builds(cls, user, build_ids, send_delete_action=True):
         """
         Delete builds specified by list of IDs
 
         :type user: models.User
         :type build_ids: list of Int
+        :param send_delete_action: boolean, set to True to let backend remove the
+            build data
         """
         to_delete = []
         no_permission = []
@@ -1317,13 +1319,13 @@ def delete_builds(cls, user, build_ids):
 
             raise BadRequest(msg)
 
-        if to_delete:
+        if to_delete and send_delete_action:
             ActionsLogic.send_delete_multiple_builds(to_delete)
 
-        for build in to_delete:
-            for build_chroot in build.build_chroots:
-                db.session.delete(build_chroot)
+        log.info("User '%s' removing builds: %s",
+                 user.username, [x.id for x in to_delete])
 
+        for build in to_delete:
             db.session.delete(build)
 
     @classmethod

frontend/coprs_frontend/coprs/logic/complex_logic.py

@@ -4,6 +4,7 @@
 import datetime
 import time
 import fnmatch
+from itertools import batched
 import flask
 import sqlalchemy
 
@@ -15,7 +16,7 @@
 from coprs import exceptions
 from coprs import cache
 from coprs.constants import DEFAULT_COPR_REPO_PRIORITY
-from coprs.exceptions import ObjectNotFound, ActionInProgressException
+from coprs.exceptions import ObjectNotFound
 from coprs.logic.builds_logic import BuildsLogic
 from coprs.logic.batches_logic import BatchesLogic
 from coprs.logic.packages_logic import PackagesLogic
@@ -97,34 +98,43 @@ def delete_copr(cls, copr, admin_action=False):
         else:
             user = flask.g.user
 
-        builds_query = BuildsLogic.get_multiple_by_copr(copr=copr)
+        builds = [x.id for x in BuildsLogic.get_multiple_by_copr(copr=copr)]
 
         if copr.persistent:
             raise exceptions.InsufficientRightsException("This project is protected against deletion.")
 
-        for build in builds_query:
+        for chunk in batched(builds, 1000):
             # Don't send delete action for each build, rather send an action to delete
             # a whole project as a part of CoprsLogic.delete_unsafe() method.
-            BuildsLogic.delete_build(user, build, send_delete_action=False)
+            BuildsLogic.delete_builds(user, chunk, send_delete_action=False)
 
         CoprsLogic.delete_unsafe(user, copr)
 
 
     @classmethod
-    def delete_expired_projects(cls):
+    def delete_expired_projects(cls, limit=None):
         query = (
             models.Copr.query
             .filter(models.Copr.delete_after.isnot(None))
             .filter(models.Copr.delete_after < datetime.datetime.now())
             .filter(models.Copr.deleted.isnot(True))
+            .limit(limit)
         )
+        deleted = 0
         for copr in query.all():
             print("deleting project '{}'".format(copr.full_name))
             try:
-               cls.delete_copr(copr, admin_action=True)
-            except ActionInProgressException as e:
+                cls.delete_copr(copr, admin_action=True)
+                deleted += 1
+            except exceptions.BadRequest as e:
+                # Ideally, we would like to catch only ActionInProgressException
+                # but the BuildsLogic.delete_builds method generalizes the
+                # exceptions and raises BadRequest.
+                if "still running" not in str(e):
+                    raise e
                 print(e)
                 print("project {} postponed".format(copr.full_name))
+        return deleted
 
 
     @classmethod

frontend/coprs_frontend/tests/test_logic/test_builds_logic.py

@@ -380,6 +380,22 @@ def test_delete_multiple_builds(self):
         with pytest.raises(NoResultFound):
             BuildsLogic.get(self.b4.id).one()
 
+    @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_builds", "f_db")
+    def test_delete_multiple_builds_chroots_deleted(self):
+        """
+        Test that the delete_builds method deletes all related chroots
+        """
+        self.b3.source_status = StatusEnum("failed")
+        self.b4.source_status = StatusEnum("failed")
+        build_ids = [self.b3.id, self.b4.id]
+
+        query = models.BuildChroot.query.filter(
+            models.BuildChroot.build_id.in_(build_ids))
+
+        assert query.count() > 0
+        BuildsLogic.delete_builds(self.u2, build_ids)
+        assert query.count() == 0
+
     @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_builds")
     def test_resubmit_build_inherit_git_hash(self):
         orig_git_hash = self.b1.build_chroots[0].git_hash