launchpad-reviewers team mailing list archive

[Michael Nelson <michael.nelson@xxxxxxxxxxxxx>]

Michael Nelson has proposed merging lp:~michael.nelson/launchpad/616331-private-builds-in-builder-history into lp:launchpad.

Requested reviews:
  Julian Edwards (julian-edwards): release-critical
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #616331 Builder history timing out
  https://bugs.launchpad.net/bugs/616331


Overview
========
We started seeing timeouts on dogfood (and apparently staging) recently for the builder history page. This branch fixes those timeouts.

Details
=======
Pre-implementation discussion (and help from stub to fix the query) can be seen at bug 616331.

The branch replaces the nested joins with normal joins. I was not able to get rid of the COALESCE.

To test:
========
bin/test -vvm test_buildfarmjob -t TestBuildFarmJobSet

I've also merged this branch on dogfood so that the page now displays fine at:

https://dogfood.launchpad.net/builders/rubidium/+history

No lint.
-- 
https://code.launchpad.net/~michael.nelson/launchpad/616331-private-builds-in-builder-history/+merge/32355
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/616331-private-builds-in-builder-history into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2010-06-16 16:01:08 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2010-08-11 17:16:00 +0000
@@ -16,7 +16,7 @@
 
 import pytz
 
-from storm.expr import And, Coalesce, Desc, Join, LeftJoin, Select
+from storm.expr import Coalesce, Desc, LeftJoin, Or
 from storm.locals import Bool, DateTime, Int, Reference, Storm
 from storm.store import Store
 
@@ -366,48 +366,35 @@
         # related archive), but not all build farm jobs will have a
         # related package build - hence the left join.
         left_join_pkg_builds = LeftJoin(
-            BuildFarmJob,
-            Join(
-                PackageBuild,
-                Archive,
-                And(PackageBuild.archive == Archive.id)),
-            PackageBuild.build_farm_job == BuildFarmJob.id)
-
-        filtered_builds = IStore(BuildFarmJob).using(
-            left_join_pkg_builds).find(BuildFarmJob, *extra_clauses)
+            PackageBuild, PackageBuild.build_farm_job == BuildFarmJob.id)
+        left_join_archive = LeftJoin(
+            Archive, PackageBuild.archive == Archive.id)
+        left_join_participation = LeftJoin(
+            TeamParticipation, TeamParticipation.teamID == Archive.ownerID)
 
         if user is None:
             # Anonymous requests don't get to see private builds at all.
-            filtered_builds = filtered_builds.find(
-                Coalesce(Archive.private, False) == False)
+            extra_clauses.append(Coalesce(Archive.private, False) == False)
 
         elif user.inTeam(getUtility(ILaunchpadCelebrities).admin):
             # Admins get to see everything.
             pass
         else:
-            # Everyone else sees a union of all public builds and the
+            # Everyone else sees all public builds and the
             # specific private builds to which they have access.
-            filtered_builds = filtered_builds.find(
-                Coalesce(Archive.private, False) == False)
-
-            user_teams_subselect = Select(
-                TeamParticipation.teamID,
-                where=And(
-                   TeamParticipation.personID == user.id,
-                   TeamParticipation.teamID == Archive.ownerID))
-            private_builds_for_user = IStore(BuildFarmJob).find(
-                BuildFarmJob,
-                PackageBuild.build_farm_job == BuildFarmJob.id,
-                PackageBuild.archive == Archive.id,
-                Archive.private == True,
-                Archive.ownerID.is_in(user_teams_subselect),
-                *extra_clauses)
-
-            filtered_builds = filtered_builds.union(
-                private_builds_for_user)
+            extra_clauses.append(
+                Or(Coalesce(Archive.private, False) == False,
+                   TeamParticipation.person == user))
+
+        filtered_builds = IStore(BuildFarmJob).using(
+            BuildFarmJob,
+            left_join_pkg_builds,
+            left_join_archive,
+            left_join_participation,
+            ).find(BuildFarmJob, *extra_clauses)
 
         filtered_builds.order_by(
             Desc(BuildFarmJob.date_finished), BuildFarmJob.id)
+        filtered_builds.config(distinct=True)
 
         return filtered_builds
-