Merge lp:~al-maisan/launchpad/qualify-subquery-507562 into lp: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:~al-maisan/launchpad/qualify-subquery-507562
Merge into: lp: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:~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.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
=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py 2010-01-14 07:10:33 +0000
+++ lib/lp/buildmaster/model/builder.py 2010-01-14 20:22:10 +0000
@@ -425,6 +425,14 @@
425425
426 :return: A candidate job.426 :return: A candidate job.
427 """427 """
428 def qualify_subquery(job_type, sub_query):
429 """Put the sub-query into a job type context."""
430 qualified_query = """
431 ((BuildQueue.job_type != %s) OR (%%s))
432 """ % sqlvalues(job_type)
433 qualified_query %= sub_query
434 return qualified_query
435
428 logger = self._getSlaveScannerLogger()436 logger = self._getSlaveScannerLogger()
429 candidate = None437 candidate = None
430438
@@ -447,7 +455,7 @@
447 extra_tables = set()455 extra_tables = set()
448 extra_queries = []456 extra_queries = []
449 job_classes = specific_job_classes()457 job_classes = specific_job_classes()
450 for job_class in job_classes.values():458 for job_type, job_class in job_classes.iteritems():
451 tables, query = job_class.addCandidateSelectionCriteria(459 tables, query = job_class.addCandidateSelectionCriteria(
452 self.processor, self.virtualized)460 self.processor, self.virtualized)
453 if query == '':461 if query == '':
@@ -459,7 +467,8 @@
459 # lower case in order to avoid duplicates in the FROM clause.467 # lower case in order to avoid duplicates in the FROM clause.
460 query_tables = query_tables.union(468 query_tables = query_tables.union(
461 set(table.lower() for table in tables))469 set(table.lower() for table in tables))
462 extra_queries.append(query)470 # The sub-query should only apply to jobs of the right type.
471 extra_queries.append(qualify_subquery(job_type, query))
463 general_query = general_query % ', '.join(query_tables)472 general_query = general_query % ', '.join(query_tables)
464 query = ' AND '.join([general_query] + extra_queries) + order_clause473 query = ' AND '.join([general_query] + extra_queries) + order_clause
465474

Subscribers

People subscribed via source and target branches

to status/vote changes: