launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/builder-history-timeout-bug-631206 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #631206 Builder:+history timeouts
  https://bugs.launchpad.net/bugs/631206

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/builder-history-timeout-bug-631206/+merge/49374

This change optimises the query that is in BuildFarmJobSet.getBuildsForBuilder().  See the bug for more commentary that I won't repeat here.  Suffice to say that the page now renders inside 2 seconds instead of timing out fairly consistently for some builders/users combos.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/builder-history-timeout-bug-631206/+merge/49374
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/builder-history-timeout-bug-631206 into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2010-10-04 21:44:32 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2011-02-11 11:34:57 +0000
@@ -15,10 +15,12 @@
 from lazr.delegates import delegates
 import pytz
 from storm.expr import (
-    Coalesce,
+    And,
     Desc,
     LeftJoin,
     Or,
+    Select,
+    Union,
     )
 from storm.locals import (
     Bool,
@@ -400,19 +402,55 @@
         # Currently only package builds can be private (via their
         # related archive), but not all build farm jobs will have a
         # related package build - hence the left join.
-        origin = [BuildFarmJob]
-        left_join_archive = [
+        origin = [
+            BuildFarmJob,
             LeftJoin(
                 PackageBuild,
                 PackageBuild.build_farm_job == BuildFarmJob.id),
-            LeftJoin(
-                Archive, PackageBuild.archive == Archive.id),
             ]
 
+        # STORM syntax has totally obfuscated this query and wasted
+        # THREE hours of my time converting perfectly good SQL syntax.  I'm
+        # really sorry if you're the poor sap who has to maintain this.
+
+        inner_privacy_query = (
+            Union(
+                Select(
+                    Archive.id,
+                    tables=(Archive,),
+                    where=(Archive.private == False)
+                    ),
+                Select(
+                    Archive.id,
+                    tables=(Archive,),
+                    where=And(
+                        Archive.private == True,
+                        Archive.ownerID.is_in(
+                            Select(
+                                TeamParticipation.teamID,
+                                where=(TeamParticipation.person == user),
+                                distinct=True
+                            )
+                        )
+                    )
+                )
+            )
+        )
+
         if user is None:
             # Anonymous requests don't get to see private builds at all.
-            origin.extend(left_join_archive)
-            extra_clauses.append(Coalesce(Archive.private, False) == False)
+            extra_clauses.append(
+                Or(
+                    PackageBuild.id == None,
+                    PackageBuild.archive_id.is_in(
+                        Select(
+                            Archive.id,
+                            tables=(Archive,),
+                            where=(Archive.private == False)
+                            )
+                        )
+                    )
+                )
 
         elif user.inTeam(getUtility(ILaunchpadCelebrities).admin):
             # Admins get to see everything.
@@ -420,13 +458,12 @@
         else:
             # Everyone else sees all public builds and the
             # specific private builds to which they have access.
-            origin.extend(left_join_archive)
-            origin.append(LeftJoin(
-                TeamParticipation,
-                TeamParticipation.teamID == Archive.ownerID))
             extra_clauses.append(
-                Or(Coalesce(Archive.private, False) == False,
-                   TeamParticipation.person == user))
+                Or(
+                    PackageBuild.id == None,
+                    PackageBuild.archive_id.is_in(inner_privacy_query)
+                    )
+                )
 
         filtered_builds = IStore(BuildFarmJob).using(*origin).find(
             BuildFarmJob, *extra_clauses)