Merge lp:~rockstar/launchpad/branch-scanner-prep into lp:launchpad/db-devel

[Paul Hummer]

[Paul Hummer (rockstar)]

Hi Edwin-

  This branch adds a JobPruner to the daily garbo script. It removes any jobs
that have a date_finished of more than thirty days. This is needed for the
next few branches that turn the scanner into a job. If we didn't do this, the
job table would get out of hand really quick.

  To test, do:
   bin/test -vvc canonical.launchpad.scripts.tests.test_garbo \
   TestGarbo.test_JobPruner

 reviewer edwin-grubbs

Cheers,
Paul

[Tim Penhey (thumper)]

Hi Paul,

I think the job pruner should be altered somewhat.

At the very least it will fail now as there are more than just branch jobs.

Also we may want to be a little more selective in the deletion and not delete
pending jobs.

If we just have a BranchJob pruner for now, we won't be enforcing our rules on
other jobs.

  review needs-fixing

review: Needs Fixing

[Tim Penhey (thumper)]

Hi Paul,

You should really add a test to make sure that branch jobs that are less than 30 days old are not deleted.

Also it seems weird that you set the date to 30 days, and the sql expects < 30 days, not <=. Is there a commit somewhere? That should at least be commented.

[Tim Penhey (thumper)]

Why do you create an actual tree? Surely this isn't needed to test that the job isn't deleted.

[Tim Penhey (thumper)]

You no longer need tempfile, and you are missing comments on your test methods.

You also have two lines between your tests, and only one at the end, c.f. PEP-8.

review: Needs Fixing

[Tim Penhey (thumper)]

review: Approve

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-01-04 15:59:17 +0000
+++ database/schema/security.cfg	2010-01-06 04:41:15 +0000
@@ -1808,6 +1808,8 @@
 public.mailinglistsubscription          = SELECT, DELETE
 public.teamparticipation                = SELECT, DELETE
 public.emailaddress                     = SELECT, UPDATE
+public.job                              = SELECT, DELETE
+public.branchjob                        = SELECT, DELETE
 
 [garbo_daily]
 type=user

=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
--- lib/canonical/launchpad/scripts/garbo.py	2009-09-15 02:22:07 +0000
+++ lib/canonical/launchpad/scripts/garbo.py	2010-01-06 04:41:15 +0000
@@ -32,10 +32,12 @@
     IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)
 from lp.bugs.model.bugnotification import BugNotification
 from lp.code.interfaces.revision import IRevisionSet
+from lp.code.model.branchjob import BranchJob
 from lp.code.model.codeimportresult import CodeImportResult
 from lp.code.model.revision import RevisionAuthor, RevisionCache
 from lp.registry.model.mailinglist import MailingListSubscription
 from lp.registry.model.person import Person
+from lp.services.job.model.job import Job
 from lp.services.scripts.base import (
     LaunchpadCronScript, SilentLaunchpadScriptFailure)
 
@@ -657,6 +659,38 @@
         self.log.debug("Removed %d rows" % num_removed)
 
 
+class BranchJobPruner(TunableLoop):
+    """Prune `BranchJob`s that are in a final state and older than a month old.
+
+    When a BranchJob is completed, it gets set to a final state.  These jobs
+    should be pruned from the database after a month.
+    """
+
+    maximum_chunk_size = 1000
+
+    def __init__(self, log, abort_time=None):
+        super(BranchJobPruner, self).__init__(log, abort_time)
+        self.job_store = IMasterStore(BranchJob)
+
+    def isDone(self):
+        return self._ids_to_remove().any() is None
+
+    def _ids_to_remove(self):
+        jobs = self.job_store.find(
+            BranchJob.id,
+            BranchJob.job == Job.id,
+            Job.date_finished < THIRTY_DAYS_AGO)
+        return jobs
+
+    def __call__(self, chunk_size):
+        chunk_size = int(chunk_size)
+        ids_to_remove = list(self._ids_to_remove()[:chunk_size])
+        num_removed = self.job_store.find(
+            BranchJob,
+            In(BranchJob.id, ids_to_remove)).remove()
+        transaction.commit()
+
+
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None # Script name for locking and database user. Override.
@@ -760,6 +794,7 @@
         MailingListSubscriptionPruner,
         PersonEmailAddressLinkChecker,
         BugNotificationPruner,
+        BranchJobPruner,
         ]
     experimental_tunable_loops = [
         PersonPruner,

=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
--- lib/canonical/launchpad/scripts/tests/test_garbo.py	2009-08-21 19:21:11 +0000
+++ lib/canonical/launchpad/scripts/tests/test_garbo.py	2010-01-06 04:41:15 +0000
@@ -38,9 +38,12 @@
     DatabaseLayer, LaunchpadScriptLayer, LaunchpadZopelessLayer)
 from lp.bugs.model.bugnotification import (
     BugNotification, BugNotificationRecipient)
+from lp.code.bzr import BranchFormat, RepositoryFormat
+from lp.code.model.branchjob import BranchJob, BranchUpgradeJob
 from lp.code.model.codeimportresult import CodeImportResult
 from lp.registry.interfaces.person import IPersonSet, PersonCreationRationale
 from lp.registry.model.person import Person
+from lp.services.job.model.job import Job
 
 
 class TestGarboScript(TestCase):
@@ -510,6 +513,63 @@
             "'[e-mail]' and 'mark' reference different Accounts")
         self.assertNotEqual(log_output.find(error_message_2), -1)
 
+    def test_BranchJobPruner(self):
+        # Garbo should remove jobs completed over 30 days ago.
+        self.useBzrBranches()
+        LaunchpadZopelessLayer.switchDbUser('testadmin')
+        store = IMasterStore(Job)
+
+        db_branch = self.factory.makeAnyBranch()
+        db_branch.branch_format = BranchFormat.BZR_BRANCH_5
+        db_branch.repository_format = RepositoryFormat.BZR_KNIT_1
+
+        branch_job = BranchUpgradeJob.create(db_branch)
+        branch_job.job.date_finished = THIRTY_DAYS_AGO
+        job_id = branch_job.job.id
+
+        self.assertEqual(
+            store.find(
+                BranchJob,
+                BranchJob.branch == db_branch.id).count(),
+                1)
+        transaction.commit()
+
+        collector = self.runDaily()
+
+        LaunchpadZopelessLayer.switchDbUser('testadmin')
+        self.assertEqual(
+            store.find(
+                BranchJob,
+                BranchJob.branch == db_branch.id).count(),
+                0)
+
+    def test_BranchJobPruner_doesnt_prune_recent_jobs(self):
+        # Check to make sure the garbo doesn't remove jobs that aren't more
+        # than thirty days old.
+        self.useBzrBranches()
+        LaunchpadZopelessLayer.switchDbUser('testadmin')
+        store = IMasterStore(Job)
+
+        db_branch = self.factory.makeAnyBranch()
+        db_branch.branch_format = BranchFormat.BZR_BRANCH_5
+        db_branch.repository_format = RepositoryFormat.BZR_KNIT_1
+
+        branch_job = BranchUpgradeJob.create(db_branch)
+        branch_job.job.date_finished = THIRTY_DAYS_AGO
+        job_id = branch_job.job.id
+
+        branch_job_newer = BranchUpgradeJob.create(db_branch)
+        job_id_newer = branch_job_newer.job.id
+        transaction.commit()
+
+        collector = self.runDaily()
+
+        LaunchpadZopelessLayer.switchDbUser('testadmin')
+        self.assertEqual(
+            store.find(
+                BranchJob).count(),
+            1)
+
 
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)