Merge lp:~gmb/launchpad/heat-garbo-hourly-bug-509195 into lp:launchpad/db-devel

Proposed by Graham Binns
Status: Merged
Merged at revision: not available
Proposed branch: lp:~gmb/launchpad/heat-garbo-hourly-bug-509195
Merge into: lp:launchpad/db-devel
Diff against target: 410 lines (+172/-52)
7 files modified
database/schema/security.cfg (+2/-1)
lib/canonical/config/schema-lazr.conf (+1/-0)
lib/canonical/launchpad/scripts/garbo.py (+19/-16)
lib/lp/bugs/configure.zcml (+1/-0)
lib/lp/bugs/doc/bug-heat.txt (+117/-32)
lib/lp/bugs/interfaces/bug.py (+11/-1)
lib/lp/bugs/model/bug.py (+21/-2)
To merge this branch: bzr merge lp:~gmb/launchpad/heat-garbo-hourly-bug-509195
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+20729@code.launchpad.net

Commit message

Bugs with out-of-date heat will now have CalculateBugHeatJobs created for them by garbo-hourly.

Description of the change

This branch adds a job to the hourly garbage collector to mop up bugs with out-of-date heat.

In this branch I've added Bug.heat_last_updated (which was in the DB already but unused), added a getBugsWithOutDatedHeat() method to IBugSet, and updated the existing (unused) BugHeatUpdater TunableLoop in garbo.py to now create CalculateBugHeatJobs for each out-of-date bug rather than updating their heat itself.

I've also added permissions to the garbo db user so that it can add new BugJobs.

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Graham,

nice banch! I have just two nitpicks, see below.

Abel

> === modified file 'lib/lp/bugs/doc/bug-heat.txt'
> --- lib/lp/bugs/doc/bug-heat.txt 2010-03-03 13:10:17 +0000
> +++ lib/lp/bugs/doc/bug-heat.txt 2010-03-05 10:13:32 +0000

[...]

> +it as a method will add jobs to calculate the heat of for all the bugs
> +whose heat is more than seven days old.

I think one of ('of', 'for') is sufficient ;)

> +
> +Before update_bug_heat is called, we'll ensure that there are no waiting
> +jobs in the bug heat calculation queue.
> +
> + >>> from lp.bugs.interfaces.bugjob import ICalculateBugHeatJobSource
> + >>> for calc_job in
getUtility(ICalculateBugHeatJobSource).iterReady():
> + ... calc_job.job.start()
> + ... calc_job.job.complete()
> +
> + >>> ready_jobs =
list(getUtility(ICalculateBugHeatJobSource).iterReady())
> + >>> len(ready_jobs)
> + 0
> +
> +We need to commit here to ensure that the bugs we've created are
> +available to the update_bug_heat script.
> +
> + >>> import transaction
> + >>> transaction.commit()
> +
> + >>> getUtility(IBugSet).getBugsWithOutdatedHeat(1).count()
> + 2
> +
> +We need to run update_bug_heat() twice to ensure that both the bugs are
> +updated.

I think you call update_bug_heat() in this doc test only once. But it
seems it should be called with chunksize=2 to ensure both bugs are
processed.

> +
> + >>> update_bug_heat(chunk_size=2)
> + DEBUG Adding CalculateBugHeatJobs for 2 Bugs (starting id: ...)
> + DEBUG Adding CalculateBugHeatJob for bug ...
> + DEBUG Adding CalculateBugHeatJob for bug ...

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFLkODxekBPhm8NrtARApukAJoCrVwFFnMkn4bR+JHA6lbUGQaZigCgkHXF
ARR5bSLClYT7Elxqmb/8X7o=
=+2Ba
-----END PGP SIGNATURE-----

Revision history for this message
Abel Deuring (adeuring) :
review: Approve (code)

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-03-01 21:59:32 +0000
3+++ database/schema/security.cfg 2010-03-05 11:49:30 +0000
4@@ -1842,8 +1842,9 @@
5 public.mailinglistsubscription = SELECT, DELETE
6 public.teamparticipation = SELECT, DELETE
7 public.emailaddress = SELECT, UPDATE
8-public.job = SELECT, DELETE
9+public.job = SELECT, INSERT, DELETE
10 public.branchjob = SELECT, DELETE
11+public.bugjob = SELECT, INSERT
12
13 [garbo_daily]
14 type=user
15
16=== modified file 'lib/canonical/config/schema-lazr.conf'
17--- lib/canonical/config/schema-lazr.conf 2010-02-24 10:18:16 +0000
18+++ lib/canonical/config/schema-lazr.conf 2010-03-05 11:49:30 +0000
19@@ -36,6 +36,7 @@
20 oops_prefix: none
21 error_dir: none
22 copy_to_zlog: false
23+max_heat_age: 7
24
25 [process_apport_blobs]
26 # The database user which will be used by this process.
27
28=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
29--- lib/canonical/launchpad/scripts/garbo.py 2010-02-02 17:44:24 +0000
30+++ lib/canonical/launchpad/scripts/garbo.py 2010-03-05 11:49:30 +0000
31@@ -31,8 +31,8 @@
32 from canonical.launchpad.webapp.interfaces import (
33 IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)
34 from lp.bugs.interfaces.bug import IBugSet
35+from lp.bugs.interfaces.bugjob import ICalculateBugHeatJobSource
36 from lp.bugs.model.bugnotification import BugNotification
37-from lp.bugs.scripts.bugheat import BugHeatCalculator
38 from lp.code.interfaces.revision import IRevisionSet
39 from lp.code.model.branchjob import BranchJob
40 from lp.code.model.codeimportresult import CodeImportResult
41@@ -698,18 +698,22 @@
42
43 maximum_chunk_size = 1000
44
45- def __init__(self, log, abort_time=None):
46+ def __init__(self, log, abort_time=None, max_heat_age=None):
47 super(BugHeatUpdater, self).__init__(log, abort_time)
48 self.transaction = transaction
49+ self.total_processed = 0
50+ self.is_done = False
51 self.offset = 0
52- self.total_updated = 0
53+ if max_heat_age is None:
54+ max_heat_age = config.calculate_bug_heat.max_heat_age
55+ self.max_heat_age = max_heat_age
56
57 def isDone(self):
58 """See `ITunableLoop`."""
59 # When the main loop has no more Bugs to process it sets
60 # offset to None. Until then, it always has a numerical
61 # value.
62- return self.offset is None
63+ return self.is_done
64
65 def __call__(self, chunk_size):
66 """Retrieve a batch of Bugs and update their heat.
67@@ -721,23 +725,23 @@
68 # trying to slice using floats or anything similarly
69 # foolish. We shouldn't have to do this.
70 chunk_size = int(chunk_size)
71-
72 start = self.offset
73 end = self.offset + chunk_size
74
75 transaction.begin()
76- # XXX 2010-01-08 gmb bug=505850:
77- # This method call should be taken out and shot as soon as
78- # we have a proper permissions system for scripts.
79- bugs = getUtility(IBugSet).dangerousGetAllBugs()[start:end]
80+ bugs = getUtility(IBugSet).getBugsWithOutdatedHeat(
81+ self.max_heat_age)[start:end]
82
83- self.offset = None
84 bug_count = bugs.count()
85 if bug_count > 0:
86 starting_id = bugs.first().id
87- self.log.debug("Updating %i Bugs (starting id: %i)" %
88+ self.log.debug(
89+ "Adding CalculateBugHeatJobs for %i Bugs (starting id: %i)" %
90 (bug_count, starting_id))
91+ else:
92+ self.is_done = True
93
94+ self.offset = None
95 for bug in bugs:
96 # We set the starting point of the next batch to the Bug
97 # id after the one we're looking at now. If there aren't any
98@@ -745,11 +749,9 @@
99 # will remain set to None.
100 start += 1
101 self.offset = start
102- self.log.debug("Updating heat for bug %s" % bug.id)
103- bug_heat_calculator = BugHeatCalculator(bug)
104- heat = bug_heat_calculator.getBugHeat()
105- bug.setHeat(heat)
106- self.total_updated += 1
107+ self.log.debug("Adding CalculateBugHeatJob for bug %s" % bug.id)
108+ getUtility(ICalculateBugHeatJobSource).create(bug)
109+ self.total_processed += 1
110 transaction.commit()
111
112
113@@ -836,6 +838,7 @@
114 OpenIDAssociationPruner,
115 OpenIDConsumerAssociationPruner,
116 RevisionCachePruner,
117+ BugHeatUpdater,
118 ]
119 experimental_tunable_loops = []
120
121
122=== modified file 'lib/lp/bugs/configure.zcml'
123--- lib/lp/bugs/configure.zcml 2010-02-19 12:31:43 +0000
124+++ lib/lp/bugs/configure.zcml 2010-03-05 11:49:30 +0000
125@@ -575,6 +575,7 @@
126 personIsAlsoNotifiedSubscriber
127 personIsSubscribedToDuplicate
128 heat
129+ heat_last_updated
130 has_patches
131 latest_patch
132 latest_patch_uploaded"/>
133
134=== modified file 'lib/lp/bugs/doc/bug-heat.txt'
135--- lib/lp/bugs/doc/bug-heat.txt 2010-03-03 13:10:17 +0000
136+++ lib/lp/bugs/doc/bug-heat.txt 2010-03-05 11:49:30 +0000
137@@ -12,54 +12,139 @@
138 >>> bug.heat
139 0
140
141+It will also have a heat_last_updated of None.
142+
143+ >>> print bug.heat_last_updated
144+ None
145+
146 The bug's heat can be set by calling its setHeat() method.
147
148 >>> bug.setHeat(42)
149 >>> bug.heat
150 42
151
152+Its heat_last_updated will also have been set.
153+
154+ >>> bug.heat_last_updated
155+ datetime.datetime(..., tzinfo=<UTC>)
156+
157+
158+Getting bugs whose heat is outdated
159+-----------------------------------
160+
161+It's possible to get the set of bugs whose heat hasn't been updated for
162+a given amount of time by calling IBugSet's getBugsWithOutdatedHeat()
163+method.
164+
165+First, we'll set the heat of all bugs so that none of them are out of
166+date.
167+
168+ >>> from lp.bugs.interfaces.bug import IBugSet
169+ >>> for bug in getUtility(IBugSet).dangerousGetAllBugs():
170+ ... bug.setHeat(0)
171+
172+If we call getBugsWithOutdatedHeat() now, the set that is returned will
173+be empty because all the bugs have been recently updated.
174+getBugsWithOutdatedHeat() takes a single parameter, max_heat_age, which
175+is the maximum age, in days, that a bug's heat can be before it gets
176+included in the returned set.
177+
178+ >>> getUtility(IBugSet).getBugsWithOutdatedHeat(1).count()
179+ 0
180+
181+IBug.setHeat() takes a timestamp parameter so that we can set the
182+heat_last_updated date manually for the purposes of testing. If we make
183+a bug's heat older than the max_heat_age that we pass to
184+getBugsWithOutdatedHeat() it will appear in the set returned by
185+getBugsWithOutdatedHeat().
186+
187+ >>> from datetime import datetime, timedelta
188+ >>> from pytz import timezone
189+ >>> old_heat_bug = factory.makeBug()
190+ >>> old_heat_bug.setHeat(
191+ ... 0, datetime.now(timezone('UTC')) - timedelta(days=2))
192+
193+ >>> outdated_bugs = getUtility(IBugSet).getBugsWithOutdatedHeat(1)
194+ >>> outdated_bugs.count()
195+ 1
196+
197+ >>> outdated_bugs[0] == old_heat_bug
198+ True
199+
200+getBugsWithOutdatedHeat() also returns bugs whose heat has never been
201+updated.
202+
203+ >>> new_bug = factory.makeBug()
204+ >>> outdated_bugs = getUtility(IBugSet).getBugsWithOutdatedHeat(1)
205+ >>> outdated_bugs.count()
206+ 2
207+
208+ >>> new_bug in outdated_bugs
209+ True
210+
211
212 The BugHeatUpdater class
213 ---------------------------
214
215-In order to calculate bug heat we need to use the BugHeatUpdater
216-class, which is designed precisely for that task. It's part of the garbo
217-module and runs as part of the garbo-daily cronjob.
218+The BugHeatUpdater class is used to create bug heat calculation jobs for
219+bugs with out-of-date heat.
220
221 >>> from canonical.launchpad.scripts.garbo import BugHeatUpdater
222 >>> from canonical.launchpad.scripts import FakeLogger
223
224- >>> update_bug_heat = BugHeatUpdater(FakeLogger())
225+ >>> update_bug_heat = BugHeatUpdater(FakeLogger(), max_heat_age=1)
226
227 BugHeatUpdater implements ITunableLoop and as such is callable. Calling
228-it as a method will update the heat for all the bugs currently held in
229-Launchpad.
230-
231-Before update_bug_heat is called, bug 1 will have no heat.
232-
233- >>> from zope.component import getUtility
234- >>> from lp.bugs.interfaces.bug import IBugSet
235- >>> bug_1 = getUtility(IBugSet).get(1)
236-
237- >>> bug_1.heat
238- 0
239-
240-We touch bug 1 to make sure its date_last_updated is recent enough (bug heat
241-decays over time).
242-
243- >>> new_comment = bug_1.newMessage(
244- ... owner=bug_1.owner, subject="...", content="...")
245- >>> import transaction ; transaction.commit()
246- >>> bug_1 = getUtility(IBugSet).get(1)
247-
248- >>> update_bug_heat(chunk_size=1)
249- DEBUG Updating 1 Bugs (starting id: ...)
250- ...
251-
252-Bug 1's heat will now be greater than 0.
253-
254- >>> bug_1.heat > 0
255- True
256+it as a method will add jobs to calculate the heat of for all the bugs
257+whose heat is more than seven days old.
258+
259+Before update_bug_heat is called, we'll ensure that there are no waiting
260+jobs in the bug heat calculation queue.
261+
262+ >>> from lp.bugs.interfaces.bugjob import ICalculateBugHeatJobSource
263+ >>> for calc_job in getUtility(ICalculateBugHeatJobSource).iterReady():
264+ ... calc_job.job.start()
265+ ... calc_job.job.complete()
266+
267+ >>> ready_jobs = list(getUtility(ICalculateBugHeatJobSource).iterReady())
268+ >>> len(ready_jobs)
269+ 0
270+
271+We need to commit here to ensure that the bugs we've created are
272+available to the update_bug_heat script.
273+
274+ >>> import transaction
275+ >>> transaction.commit()
276+
277+ >>> getUtility(IBugSet).getBugsWithOutdatedHeat(1).count()
278+ 2
279+
280+We need to run update_bug_heat() twice to ensure that both the bugs are
281+updated.
282+
283+ >>> update_bug_heat(chunk_size=2)
284+ DEBUG Adding CalculateBugHeatJobs for 2 Bugs (starting id: ...)
285+ DEBUG Adding CalculateBugHeatJob for bug ...
286+ DEBUG Adding CalculateBugHeatJob for bug ...
287+
288+There will now be two CalculateBugHeatJobs in the queue.
289+
290+ >>> ready_jobs = list(getUtility(ICalculateBugHeatJobSource).iterReady())
291+ >>> len(ready_jobs)
292+ 2
293+
294+Running them will update the bugs' heat.
295+
296+ >>> for calc_job in getUtility(ICalculateBugHeatJobSource).iterReady():
297+ ... calc_job.job.start()
298+ ... calc_job.run()
299+ ... calc_job.job.complete()
300+
301+IBugSet.getBugsWithOutdatedHeat() will now return an empty set since all
302+the bugs have been updated.
303+
304+ >>> getUtility(IBugSet).getBugsWithOutdatedHeat(1).count()
305+ 0
306
307
308 Caculating the maximum heat for a target
309
310=== modified file 'lib/lp/bugs/interfaces/bug.py'
311--- lib/lp/bugs/interfaces/bug.py 2010-02-27 20:20:03 +0000
312+++ lib/lp/bugs/interfaces/bug.py 2010-03-05 11:49:30 +0000
313@@ -320,6 +320,8 @@
314 heat = exported(
315 Int(title=_("The 'heat' of the bug"),
316 required=False, readonly=True))
317+ heat_last_updated = Datetime(
318+ title=_('Heat Last Updated'), required=False, readonly=True)
319
320 # Adding related BugMessages provides a hook for getting at
321 # BugMessage.visible when building bug comments.
322@@ -778,7 +780,7 @@
323 if the user is the owner or an admin.
324 """
325
326- def setHeat(heat):
327+ def setHeat(heat, timestamp=None):
328 """Set the heat for the bug."""
329
330 class InvalidDuplicateValue(Exception):
331@@ -1032,6 +1034,14 @@
332 # Note, this method should go away when we have a proper
333 # permissions system for scripts.
334
335+ def getBugsWithOutdatedHeat(max_heat_age):
336+ """Return the set of bugs whose heat is out of date.
337+
338+ :param max_heat_age: The maximum age, in days, that a bug's heat
339+ can be before it is included in the
340+ returned set.
341+ """
342+
343
344 class IFileBugData(Interface):
345 """A class containing extra data to be used when filing a bug."""
346
347=== modified file 'lib/lp/bugs/model/bug.py'
348--- lib/lp/bugs/model/bug.py 2010-02-28 13:41:18 +0000
349+++ lib/lp/bugs/model/bug.py 2010-03-05 11:49:30 +0000
350@@ -22,7 +22,9 @@
351 import operator
352 import re
353 from cStringIO import StringIO
354+from datetime import datetime, timedelta
355 from email.Utils import make_msgid
356+from pytz import timezone
357
358 from zope.contenttype import guess_content_type
359 from zope.component import getUtility
360@@ -33,7 +35,7 @@
361 from sqlobject import SQLMultipleJoin, SQLRelatedJoin
362 from sqlobject import SQLObjectNotFound
363 from storm.expr import (
364- And, Count, Func, In, LeftJoin, Max, Not, Select, SQLRaw, Union)
365+ And, Count, Func, In, LeftJoin, Max, Not, Or, Select, SQLRaw, Union)
366 from storm.store import EmptyResultSet, Store
367
368 from lazr.lifecycle.event import (
369@@ -260,6 +262,7 @@
370 users_affected_count = IntCol(notNull=True, default=0)
371 users_unaffected_count = IntCol(notNull=True, default=0)
372 heat = IntCol(notNull=True, default=0)
373+ heat_last_updated = UtcDateTimeCol(default=None)
374 latest_patch_uploaded = UtcDateTimeCol(default=None)
375
376 @property
377@@ -1531,9 +1534,13 @@
378
379 return not subscriptions_from_dupes.is_empty()
380
381- def setHeat(self, heat):
382+ def setHeat(self, heat, timestamp=None):
383 """See `IBug`."""
384+ if timestamp is None:
385+ timestamp = UTC_NOW
386+
387 self.heat = heat
388+ self.heat_last_updated = timestamp
389 for task in self.bugtasks:
390 task.target.recalculateMaxBugHeat()
391
392@@ -1790,6 +1797,18 @@
393 result_set = store.find(Bug)
394 return result_set.order_by('id')
395
396+ def getBugsWithOutdatedHeat(self, max_heat_age):
397+ """See `IBugSet`."""
398+ store = IStore(Bug)
399+ last_updated_cutoff = (
400+ datetime.now(timezone('UTC')) -
401+ timedelta(days=max_heat_age))
402+ last_updated_clause = Or(
403+ Bug.heat_last_updated < last_updated_cutoff,
404+ Bug.heat_last_updated == None)
405+
406+ return store.find(Bug, last_updated_clause).order_by('id')
407+
408
409 class BugAffectsPerson(SQLBase):
410 """A bug is marked as affecting a user."""

Subscribers

People subscribed via source and target branches

to status/vote changes: