Launchpad itself

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

  1. branch-scanner-prep
  2. Merge into db-devel
Proposed by Paul Hummer
Status: Merged
Approved by: Tim Penhey
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~rockstar/launchpad/branch-scanner-prep
Merge into: lp:launchpad/db-devel
Diff against target: 157 lines (+97/-0)
3 files modified
database/schema/security.cfg (+2/-0)
lib/canonical/launchpad/scripts/garbo.py (+35/-0)
lib/canonical/launchpad/scripts/tests/test_garbo.py (+60/-0)
To merge this branch: bzr merge lp:~rockstar/launchpad/branch-scanner-prep
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Edwin Grubbs Pending
Review via email: mp+16260@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) wrote :

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

Revision history for this message
Tim Penhey (thumper) wrote :

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
Revision history for this message
Tim Penhey (thumper) wrote :

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.

Revision history for this message
Tim Penhey (thumper) wrote :

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

Revision history for this message
Tim Penhey (thumper) wrote :

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
Revision history for this message
Tim Penhey (thumper) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2010-01-04 15:59:17 +0000
3+++ database/schema/security.cfg 2010-01-06 04:41:15 +0000
4@@ -1808,6 +1808,8 @@
5 public.mailinglistsubscription = SELECT, DELETE
6 public.teamparticipation = SELECT, DELETE
7 public.emailaddress = SELECT, UPDATE
8+public.job = SELECT, DELETE
9+public.branchjob = SELECT, DELETE
10
11 [garbo_daily]
12 type=user
13
14=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
15--- lib/canonical/launchpad/scripts/garbo.py 2009-09-15 02:22:07 +0000
16+++ lib/canonical/launchpad/scripts/garbo.py 2010-01-06 04:41:15 +0000
17@@ -32,10 +32,12 @@
18 IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)
19 from lp.bugs.model.bugnotification import BugNotification
20 from lp.code.interfaces.revision import IRevisionSet
21+from lp.code.model.branchjob import BranchJob
22 from lp.code.model.codeimportresult import CodeImportResult
23 from lp.code.model.revision import RevisionAuthor, RevisionCache
24 from lp.registry.model.mailinglist import MailingListSubscription
25 from lp.registry.model.person import Person
26+from lp.services.job.model.job import Job
27 from lp.services.scripts.base import (
28 LaunchpadCronScript, SilentLaunchpadScriptFailure)
29
30@@ -657,6 +659,38 @@
31 self.log.debug("Removed %d rows" % num_removed)
32
33
34+class BranchJobPruner(TunableLoop):
35+ """Prune `BranchJob`s that are in a final state and older than a month old.
36+
37+ When a BranchJob is completed, it gets set to a final state. These jobs
38+ should be pruned from the database after a month.
39+ """
40+
41+ maximum_chunk_size = 1000
42+
43+ def __init__(self, log, abort_time=None):
44+ super(BranchJobPruner, self).__init__(log, abort_time)
45+ self.job_store = IMasterStore(BranchJob)
46+
47+ def isDone(self):
48+ return self._ids_to_remove().any() is None
49+
50+ def _ids_to_remove(self):
51+ jobs = self.job_store.find(
52+ BranchJob.id,
53+ BranchJob.job == Job.id,
54+ Job.date_finished < THIRTY_DAYS_AGO)
55+ return jobs
56+
57+ def __call__(self, chunk_size):
58+ chunk_size = int(chunk_size)
59+ ids_to_remove = list(self._ids_to_remove()[:chunk_size])
60+ num_removed = self.job_store.find(
61+ BranchJob,
62+ In(BranchJob.id, ids_to_remove)).remove()
63+ transaction.commit()
64+
65+
66 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
67 """Abstract base class to run a collection of TunableLoops."""
68 script_name = None # Script name for locking and database user. Override.
69@@ -760,6 +794,7 @@
70 MailingListSubscriptionPruner,
71 PersonEmailAddressLinkChecker,
72 BugNotificationPruner,
73+ BranchJobPruner,
74 ]
75 experimental_tunable_loops = [
76 PersonPruner,
77
78=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
79--- lib/canonical/launchpad/scripts/tests/test_garbo.py 2009-08-21 19:21:11 +0000
80+++ lib/canonical/launchpad/scripts/tests/test_garbo.py 2010-01-06 04:41:15 +0000
81@@ -38,9 +38,12 @@
82 DatabaseLayer, LaunchpadScriptLayer, LaunchpadZopelessLayer)
83 from lp.bugs.model.bugnotification import (
84 BugNotification, BugNotificationRecipient)
85+from lp.code.bzr import BranchFormat, RepositoryFormat
86+from lp.code.model.branchjob import BranchJob, BranchUpgradeJob
87 from lp.code.model.codeimportresult import CodeImportResult
88 from lp.registry.interfaces.person import IPersonSet, PersonCreationRationale
89 from lp.registry.model.person import Person
90+from lp.services.job.model.job import Job
91
92
93 class TestGarboScript(TestCase):
94@@ -510,6 +513,63 @@
95 "'mark@example.com' and 'mark' reference different Accounts")
96 self.assertNotEqual(log_output.find(error_message_2), -1)
97
98+ def test_BranchJobPruner(self):
99+ # Garbo should remove jobs completed over 30 days ago.
100+ self.useBzrBranches()
101+ LaunchpadZopelessLayer.switchDbUser('testadmin')
102+ store = IMasterStore(Job)
103+
104+ db_branch = self.factory.makeAnyBranch()
105+ db_branch.branch_format = BranchFormat.BZR_BRANCH_5
106+ db_branch.repository_format = RepositoryFormat.BZR_KNIT_1
107+
108+ branch_job = BranchUpgradeJob.create(db_branch)
109+ branch_job.job.date_finished = THIRTY_DAYS_AGO
110+ job_id = branch_job.job.id
111+
112+ self.assertEqual(
113+ store.find(
114+ BranchJob,
115+ BranchJob.branch == db_branch.id).count(),
116+ 1)
117+ transaction.commit()
118+
119+ collector = self.runDaily()
120+
121+ LaunchpadZopelessLayer.switchDbUser('testadmin')
122+ self.assertEqual(
123+ store.find(
124+ BranchJob,
125+ BranchJob.branch == db_branch.id).count(),
126+ 0)
127+
128+ def test_BranchJobPruner_doesnt_prune_recent_jobs(self):
129+ # Check to make sure the garbo doesn't remove jobs that aren't more
130+ # than thirty days old.
131+ self.useBzrBranches()
132+ LaunchpadZopelessLayer.switchDbUser('testadmin')
133+ store = IMasterStore(Job)
134+
135+ db_branch = self.factory.makeAnyBranch()
136+ db_branch.branch_format = BranchFormat.BZR_BRANCH_5
137+ db_branch.repository_format = RepositoryFormat.BZR_KNIT_1
138+
139+ branch_job = BranchUpgradeJob.create(db_branch)
140+ branch_job.job.date_finished = THIRTY_DAYS_AGO
141+ job_id = branch_job.job.id
142+
143+ branch_job_newer = BranchUpgradeJob.create(db_branch)
144+ job_id_newer = branch_job_newer.job.id
145+ transaction.commit()
146+
147+ collector = self.runDaily()
148+
149+ LaunchpadZopelessLayer.switchDbUser('testadmin')
150+ self.assertEqual(
151+ store.find(
152+ BranchJob).count(),
153+ 1)
154+
155
156 def test_suite():
157 return unittest.TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: