Launchpad itself

Merge lp:~rvb/launchpad/builders-timeout-903827-2 into lp:launchpad

  1. builders-timeout-903827-2
  2. Merge into devel
Proposed by Raphaël Badin
Status: Merged
Approved by: Raphaël Badin
Approved revision: no longer in the source branch.
Merged at revision: 14654
Proposed branch: lp:~rvb/launchpad/builders-timeout-903827-2
Merge into: lp:launchpad
Prerequisite: lp:~rvb/launchpad/builder-history-lfa
Diff against target: 312 lines (+64/-26)
7 files modified
lib/lp/buildmaster/doc/buildqueue.txt (+3/-0)
lib/lp/buildmaster/manager.py (+7/-2)
lib/lp/buildmaster/model/builder.py (+7/-1)
lib/lp/buildmaster/model/buildqueue.py (+16/-2)
lib/lp/code/model/sourcepackagerecipebuild.py (+5/-3)
lib/lp/soyuz/browser/builder.py (+12/-3)
lib/lp/soyuz/browser/tests/test_builder.py (+14/-15)
To merge this branch: bzr merge lp:~rvb/launchpad/builders-timeout-903827-2
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+87620@code.launchpad.net

Commit message

[r=jcsackett][bug=903827] Make buildqueue.specific_job and builder.currentjob cached properties.

Description of the change

This branch makes buildqueue.specific_job and builder.currentjob cached properties. builder.currentjob is eager loaded in lib/lp/soyuz/browser/builder and buildqueue.specific_job (which should not change over time) is loaded in preloadSpecificJobData.

= Tests =
(modified)
test_builder test_builders_binary_package_build_query_count
test_builder test_builders_recipe_build_query_count
test_builder test_builders_translation_template_build_query_count

= Q/A =
The number of repeated statements of https://launchpad.net/builders should diminish a great deal. It's currently around in [300 - 600] depending on the type of builds building. Note that the repeated statement issued to fetch buildqueue records will still be there.

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

This looks like a good improvement.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/doc/buildqueue.txt'
2--- lib/lp/buildmaster/doc/buildqueue.txt 2011-12-30 06:14:56 +0000
3+++ lib/lp/buildmaster/doc/buildqueue.txt 2012-01-06 15:32:25 +0000
4@@ -13,6 +13,7 @@
5 collected) the BuildQueue record representing it is removed.
6
7 >>> from lp.services.webapp.testing import verifyObject
8+ >>> from lp.services.propertycache import get_property_cache
9 >>> from lp.buildmaster.interfaces.buildqueue import (
10 ... IBuildQueue, IBuildQueueSet)
11
12@@ -135,6 +136,7 @@
13
14 >>> job.reset()
15
16+ >>> del get_property_cache(bob).currentjob
17 >>> print bob.currentjob
18 None
19
20@@ -154,6 +156,7 @@
21
22 >>> job.markAsBuilding(bob)
23
24+ >>> del get_property_cache(bob).currentjob
25 >>> bob.currentjob == job
26 True
27
28
29=== modified file 'lib/lp/buildmaster/manager.py'
30--- lib/lp/buildmaster/manager.py 2012-01-03 12:17:29 +0000
31+++ lib/lp/buildmaster/manager.py 2012-01-06 15:32:25 +0000
32@@ -1,4 +1,4 @@
33-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
34+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
35 # GNU Affero General Public License version 3 (see the file LICENSE).
36
37 """Soyuz buildd slave manager logic."""
38@@ -34,6 +34,7 @@
39 BuildBehaviorMismatch,
40 )
41 from lp.buildmaster.model.builder import Builder
42+from lp.services.propertycache import get_property_cache
43 from lp.services.database.transaction_policy import DatabaseTransactionPolicy
44
45
46@@ -50,7 +51,9 @@
47 def assessFailureCounts(builder, fail_notes):
48 """View builder/job failure_count and work out which needs to die. """
49 # builder.currentjob hides a complicated query, don't run it twice.
50- # See bug 623281.
51+ # See bug 623281 (Note that currentjob is a cachedproperty).
52+
53+ del get_property_cache(builder).currentjob
54 current_job = builder.currentjob
55 if current_job is None:
56 job_failure_count = 0
57@@ -64,6 +67,7 @@
58 # we can do is try them both again, and hope that the job
59 # runs against a different builder.
60 current_job.reset()
61+ del get_property_cache(builder).currentjob
62 return
63
64 if builder.failure_count > job_failure_count:
65@@ -96,6 +100,7 @@
66 # but that would cause us to query the slave for its status
67 # again, and if the slave is non-responsive it holds up the
68 # next buildd scan.
69+ del get_property_cache(builder).currentjob
70
71
72 class SlaveScanner:
73
74=== modified file 'lib/lp/buildmaster/model/builder.py'
75--- lib/lp/buildmaster/model/builder.py 2012-01-03 12:11:57 +0000
76+++ lib/lp/buildmaster/model/builder.py 2012-01-06 15:32:25 +0000
77@@ -431,6 +431,7 @@
78
79 def _getCurrentBuildBehavior(self):
80 """Return the current build behavior."""
81+ self._clean_currentjob_cache()
82 if not safe_hasattr(self, '_current_build_behavior'):
83 self._current_build_behavior = None
84
85@@ -479,10 +480,12 @@
86 def gotFailure(self):
87 """See `IBuilder`."""
88 self.failure_count += 1
89+ self._clean_currentjob_cache()
90
91 def resetFailureCount(self):
92 """See `IBuilder`."""
93 self.failure_count = 0
94+ self._clean_currentjob_cache()
95
96 def rescueIfLost(self, logger=None):
97 """See `IBuilder`."""
98@@ -498,11 +501,14 @@
99
100 # XXX 2010-08-24 Julian bug=623281
101 # This should not be a property! It's masking a complicated query.
102- @property
103+ @cachedproperty
104 def currentjob(self):
105 """See IBuilder"""
106 return getUtility(IBuildQueueSet).getByBuilder(self)
107
108+ def _clean_currentjob_cache(self):
109+ del get_property_cache(self).currentjob
110+
111 def requestAbort(self):
112 """See IBuilder."""
113 return self.slave.abort()
114
115=== modified file 'lib/lp/buildmaster/model/buildqueue.py'
116--- lib/lp/buildmaster/model/buildqueue.py 2012-01-02 11:21:14 +0000
117+++ lib/lp/buildmaster/model/buildqueue.py 2012-01-06 15:32:25 +0000
118@@ -1,4 +1,4 @@
119-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
120+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
121 # GNU Affero General Public License version 3 (see the file LICENSE).
122
123 # pylint: disable-msg=E0611,W0212
124@@ -53,6 +53,10 @@
125 )
126 from lp.services.job.interfaces.job import JobStatus
127 from lp.services.job.model.job import Job
128+from lp.services.propertycache import (
129+ cachedproperty,
130+ get_property_cache,
131+ )
132 from lp.services.webapp.interfaces import (
133 DEFAULT_FLAVOR,
134 IStoreSelector,
135@@ -138,12 +142,15 @@
136 """See `IBuildQueue`."""
137 return IBuildFarmJobBehavior(self.specific_job)
138
139- @property
140+ @cachedproperty
141 def specific_job(self):
142 """See `IBuildQueue`."""
143 specific_class = specific_job_classes()[self.job_type]
144 return specific_class.getByJob(self.job)
145
146+ def _clear_specific_job_cache(self):
147+ del get_property_cache(self).specific_job
148+
149 @staticmethod
150 def preloadSpecificJobData(queues):
151 key = attrgetter('job_type')
152@@ -158,6 +165,12 @@
153 if len(list(specific_jobs)) == 0:
154 continue
155 specific_class.preloadJobsData(specific_jobs)
156+ specific_jobs_dict = dict(
157+ (specific_job.job, specific_job)
158+ for specific_job in specific_jobs)
159+ for queue in queue_subset:
160+ cache = get_property_cache(queue)
161+ cache.specific_job = specific_jobs_dict[queue.job]
162
163 @property
164 def date_started(self):
165@@ -180,6 +193,7 @@
166 SQLBase.destroySelf(self)
167 specific_job.cleanUp()
168 job.destroySelf()
169+ self._clear_specific_job_cache()
170
171 def manualScore(self, value):
172 """See `IBuildQueue`."""
173
174=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
175--- lib/lp/code/model/sourcepackagerecipebuild.py 2012-01-06 15:32:25 +0000
176+++ lib/lp/code/model/sourcepackagerecipebuild.py 2012-01-06 15:32:25 +0000
177@@ -1,4 +1,4 @@
178-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
179+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
180 # GNU Affero General Public License version 3 (see the file LICENSE).
181
182 # pylint: disable-msg=F0401,E1002
183@@ -291,10 +291,12 @@
184 # Circular imports.
185 from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
186 from lp.services.librarian.model import LibraryFileAlias
187+ from lp.buildmaster.model.buildfarmjob import BuildFarmJob
188 package_builds = load_related(
189 PackageBuild, builds, ['package_build_id'])
190- build_farm_jobs = [
191- build.build_farm_job for build in builds]
192+ build_farm_jobs = load_related(
193+ BuildFarmJob, [build.package_build for build in builds],
194+ ['build_farm_job_id'])
195 load_related(LibraryFileAlias, build_farm_jobs, ['log_id'])
196 archives = load_related(Archive, package_builds, ['archive_id'])
197 load_related(Person, archives, ['ownerID'])
198
199=== modified file 'lib/lp/soyuz/browser/builder.py'
200--- lib/lp/soyuz/browser/builder.py 2012-01-02 11:21:14 +0000
201+++ lib/lp/soyuz/browser/builder.py 2012-01-06 15:32:25 +0000
202@@ -1,4 +1,4 @@
203-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
204+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
205 # GNU Affero General Public License version 3 (see the file LICENSE).
206
207 """Browser views for builders."""
208@@ -44,7 +44,10 @@
209 from lp.buildmaster.model.buildqueue import BuildQueue
210 from lp.services.database.decoratedresultset import DecoratedResultSet
211 from lp.services.database.lpstorm import IStore
212-from lp.services.propertycache import cachedproperty
213+from lp.services.propertycache import (
214+ cachedproperty,
215+ get_property_cache,
216+ )
217 from lp.services.webapp import (
218 ApplicationMenu,
219 canonical_url,
220@@ -148,11 +151,17 @@
221 def builders(self):
222 """All active builders"""
223 def do_eager_load(builders):
224- # Prefetch the jobs' data.
225+ # Populate builders' currentjob cachedproperty.
226 queues = IStore(BuildQueue).find(
227 BuildQueue,
228 BuildQueue.builderID.is_in(
229 builder.id for builder in builders))
230+ queue_builders = dict(
231+ (queue.builderID, queue) for queue in queues)
232+ for builder in builders:
233+ cache = get_property_cache(builder)
234+ cache.currentjob = queue_builders.get(builder.id, None)
235+ # Prefetch the jobs' data.
236 BuildQueue.preloadSpecificJobData(queues)
237
238 return list(DecoratedResultSet(
239
240=== modified file 'lib/lp/soyuz/browser/tests/test_builder.py'
241--- lib/lp/soyuz/browser/tests/test_builder.py 2012-01-06 15:32:25 +0000
242+++ lib/lp/soyuz/browser/tests/test_builder.py 2012-01-06 15:32:25 +0000
243@@ -1,4 +1,4 @@
244-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
245+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
246 # GNU Affero General Public License version 3 (see the file LICENSE).
247
248 """Tests for the lp.soyuz.browser.builder module."""
249@@ -67,14 +67,10 @@
250
251 layer = LaunchpadFunctionalLayer
252
253- # XXX rvb: the 3 additional queries per build are the result of the calls
254- # to:
255- # - builder.currentjob
256- # - buildqueue.specific_job
257- # These could be converted into cachedproperty and pre-populated in
258- # bulk but several tests assert that the value returned by these
259- # these properties are up to date. Since they are not really expensive
260- # to compute I'll leave them as regular properties for now.
261+ # XXX rvb: the query issued per build is the result of the call to
262+ # build.buildqueue_record. It was decided not to make it a cachedproperty
263+ # because the code relies on the fact that this property always returns
264+ # the current record.
265
266 def test_builders_binary_package_build_query_count(self):
267 def create_build():
268@@ -82,12 +78,13 @@
269 queue = build.queueBuild()
270 queue.markAsBuilding(build.builder)
271
272+ nb_objects = 2
273 recorder1, recorder2 = record_two_runs(
274- builders_homepage_render, create_build, 2)
275+ builders_homepage_render, create_build, nb_objects)
276
277 self.assertThat(
278 recorder2,
279- HasQueryCount(LessThan(recorder1.count + 3 * 2 + 1)))
280+ HasQueryCount(LessThan(recorder1.count + 1 * nb_objects + 1)))
281
282 def test_builders_recipe_build_query_count(self):
283 def create_build():
284@@ -95,12 +92,13 @@
285 queue = build.queueBuild()
286 queue.markAsBuilding(build.builder)
287
288+ nb_objects = 2
289 recorder1, recorder2 = record_two_runs(
290- builders_homepage_render, create_build, 2)
291+ builders_homepage_render, create_build, nb_objects)
292
293 self.assertThat(
294 recorder2,
295- HasQueryCount(LessThan(recorder1.count + 4 * 2 + 1)))
296+ HasQueryCount(LessThan(recorder1.count + 1 * nb_objects + 1)))
297
298 def test_builders_translation_template_build_query_count(self):
299 def create_build():
300@@ -114,9 +112,10 @@
301 queue = queueset.get(job_id)
302 queue.markAsBuilding(self.factory.makeBuilder())
303
304+ nb_objects = 2
305 recorder1, recorder2 = record_two_runs(
306- builders_homepage_render, create_build, 2)
307+ builders_homepage_render, create_build, nb_objects)
308
309 self.assertThat(
310 recorder2,
311- HasQueryCount(LessThan(recorder1.count + 3 * 2 + 1)))
312+ HasQueryCount(LessThan(recorder1.count + 1 * nb_objects + 1)))