launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-1422562 into lp:launchpad.

Commit message:
Only retrieve the top 10 items in the build queue at a time, fixing buildd-manager performance with deep queues.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1422562 in Launchpad itself: "Builder._findBuildCandidate is very slow"
  https://bugs.launchpad.net/launchpad/+bug/1422562

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1422562/+merge/249920

Only retrieve the top 10 items in the build queue at a time.

Previously we'd retrieve every candidate BuildQueue.id in order and
invoke postprocessCandidate on each until we found one that passed. But
in the real world there's always a valid candidate near the top of the
queue, postprocessCandidate unqueues any jobs that it rejects, and we
want bounded per-cycle query times anyway. If we do end up with 1000
unusable jobs at the top of the queue, we just have to wait a minute or
so for them to be killed, 10 at a time.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-1422562 into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py	2014-06-26 12:52:06 +0000
+++ lib/lp/buildmaster/model/builder.py	2015-02-17 05:37:28 +0000
@@ -19,8 +19,14 @@
     StringCol,
     )
 from storm.expr import (
+    And,
     Coalesce,
     Count,
+    Desc,
+    Exists,
+    Or,
+    Select,
+    SQL,
     Sum,
     )
 from storm.properties import Int
@@ -43,6 +49,7 @@
     )
 from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSet
 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
+from lp.buildmaster.model.buildfarmjob import BuildFarmJob
 from lp.buildmaster.model.buildqueue import (
     BuildQueue,
     specific_build_farm_job_sources,
@@ -244,45 +251,38 @@
             qualified_query %= sub_query
             return qualified_query
 
-        logger = self._getSlaveScannerLogger()
-        candidate = None
-
-        general_query = """
-            SELECT buildqueue.id FROM buildqueue, buildfarmjob
-            WHERE
-                buildfarmjob.id = buildqueue.build_farm_job
-                AND buildqueue.status = %s
-                AND (
-                    -- The processor values either match or the candidate
-                    -- job is processor-independent.
-                    buildqueue.processor IN (
-                        SELECT processor FROM BuilderProcessor
-                        WHERE builder = %s) OR
-                    buildqueue.processor IS NULL)
-                AND buildqueue.virtualized = %s
-                AND buildqueue.builder IS NULL
-        """ % sqlvalues(
-            BuildQueueStatus.WAITING, self, self.virtualized)
-        order_clause = " ORDER BY buildqueue.lastscore DESC, buildqueue.id"
-
-        extra_queries = []
+        job_type_conditions = []
         job_sources = specific_build_farm_job_sources()
         for job_type, job_source in job_sources.iteritems():
             query = job_source.addCandidateSelectionCriteria(
                 self.processor, self.virtualized)
-            if query == '':
-                # This job class does not need to refine candidate jobs
-                # further.
-                continue
-
-            # The sub-query should only apply to jobs of the right type.
-            extra_queries.append(qualify_subquery(job_type, query))
-        query = ' AND '.join([general_query] + extra_queries) + order_clause
+            if query:
+                job_type_conditions.append(
+                    Or(
+                        BuildFarmJob.job_type != job_type,
+                        Exists(SQL(query))))
 
         store = IStore(self.__class__)
-        candidate_jobs = store.execute(query).get_all()
+        candidate_jobs = store.using(BuildQueue, BuildFarmJob).find(
+            (BuildQueue.id,),
+            BuildFarmJob.id == BuildQueue._build_farm_job_id,
+            BuildQueue.status == BuildQueueStatus.WAITING,
+            Or(
+                BuildQueue.processorID.is_in(Select(
+                    BuilderProcessor.processor_id, tables=[BuilderProcessor],
+                    where=BuilderProcessor.builder == self)),
+                BuildQueue.processor == None),
+            BuildQueue.virtualized == self.virtualized,
+            BuildQueue.builder == None,
+            And(*job_type_conditions)
+            ).order_by(Desc(BuildQueue.lastscore), BuildQueue.id)
 
-        for (candidate_id,) in candidate_jobs:
+        logger = self._getSlaveScannerLogger()
+        # Only try the first handful of jobs. It's much easier on the
+        # database, the chance of a large prefix of the queue being
+        # bad candidates is negligible, and we want reasonably bounded
+        # per-cycle performance even if the prefix is large.
+        for (candidate_id,) in candidate_jobs[:10]:
             candidate = getUtility(IBuildQueueSet).get(candidate_id)
             job_source = job_sources[
                 removeSecurityProxy(candidate)._build_farm_job.job_type]