Merge lp://staging/~al-maisan/launchpad/qualify-subquery-507562 into lp://staging/launchpad/db-devel

Proposed by Muharem Hrnjadovic
Status: Merged
Approved by: Julian Edwards
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://staging/~al-maisan/launchpad/qualify-subquery-507562
Merge into: lp://staging/launchpad/db-devel
Diff against target: 36 lines (+11/-2)
1 file modified
lib/lp/buildmaster/model/builder.py (+11/-2)
To merge this branch: bzr merge lp://staging/~al-maisan/launchpad/qualify-subquery-507562
Reviewer Review Type Date Requested Status
Julian Edwards (community) code Approve
Canonical Launchpad Engineering Pending
Review via email: mp+17399@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Hello there!

The selection of candidate jobs for idle builders in the build farm uses a
general query part that may select too many candidates.
Thus, all build farm job type classes get a chance to add sub-queries that
further refine or narrow down the set of selected candidate jobs.

However, these sub-queries need to be put into context so that they apply only
to the jobs beloging to the job class that contributed the sub-query.

That is what the branch at hand does.

Please note that all _findBuildCandidate() tests (we have a pretty extensive
test coverage of the build farm candidate job selection) pass.

Test to run:

    bin/test -vv -t build

No "make lint" errors or warnings.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Approved with the readability improvement we discussed in person.

Cheers
J

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/model/builder.py'
2--- lib/lp/buildmaster/model/builder.py 2010-01-14 07:10:33 +0000
3+++ lib/lp/buildmaster/model/builder.py 2010-01-14 20:22:10 +0000
4@@ -425,6 +425,14 @@
5
6 :return: A candidate job.
7 """
8+ def qualify_subquery(job_type, sub_query):
9+ """Put the sub-query into a job type context."""
10+ qualified_query = """
11+ ((BuildQueue.job_type != %s) OR (%%s))
12+ """ % sqlvalues(job_type)
13+ qualified_query %= sub_query
14+ return qualified_query
15+
16 logger = self._getSlaveScannerLogger()
17 candidate = None
18
19@@ -447,7 +455,7 @@
20 extra_tables = set()
21 extra_queries = []
22 job_classes = specific_job_classes()
23- for job_class in job_classes.values():
24+ for job_type, job_class in job_classes.iteritems():
25 tables, query = job_class.addCandidateSelectionCriteria(
26 self.processor, self.virtualized)
27 if query == '':
28@@ -459,7 +467,8 @@
29 # lower case in order to avoid duplicates in the FROM clause.
30 query_tables = query_tables.union(
31 set(table.lower() for table in tables))
32- extra_queries.append(query)
33+ # The sub-query should only apply to jobs of the right type.
34+ extra_queries.append(qualify_subquery(job_type, query))
35 general_query = general_query % ', '.join(query_tables)
36 query = ' AND '.join([general_query] + extra_queries) + order_clause
37

Subscribers

People subscribed via source and target branches

to status/vote changes: