launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/localpackagediffs-time-out-bug-798301-alt into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #798301 in Launchpad itself: "Time-out on +localpackagediffs"
  https://bugs.launchpad.net/launchpad/+bug/798301

For more details, see:
https://code.launchpad.net/~allenap/launchpad/localpackagediffs-time-out-bug-798301-alt/+merge/65521

This branch does three related and small things, all related to
most_recent_publications(). This function is intended to bulk load
several attributes of DistroSeriesDifference, and its behaviour is
derived from that of DistroSeries.getPublishedSources() and
Archive.getPublishedSources(). The former is deprecated but still
heavily in use, and the latter is its successor.

The things:

- DistroSeries.getPublishedSources() looks in all Archives that have a
  purpose in MAIN_ARCHIVE_PURPOSES, namely PRIMARY, PARTNER or DEBUG,
  and most_recent_publications() has been adjusted to also do this
  match. Previously it only matched PRIMARY archives.

- It constrains SourcePackagePublishingHistory.distroseries to match
  the DistroSeries currently under consideration. Again, this is to
  match the behaviour in getPublishedSources().

- The big one: stop using the "wrong" index. PostgreSQL comes up with
  some stupid query plans when it attempts to use an (archive, status)
  index on SPPH. Joining on (SPPH.archive + 0 == Archive.id) prevents
  the use of this index.

-- 
https://code.launchpad.net/~allenap/launchpad/localpackagediffs-time-out-bug-798301-alt/+merge/65521
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/localpackagediffs-time-out-bug-798301-alt into lp:launchpad.

=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py	2011-06-14 13:47:51 +0000
+++ lib/lp/registry/model/distroseriesdifference.py	2011-06-22 15:46:04 +0000
@@ -83,10 +83,10 @@
     get_property_cache,
     )
 from lp.soyuz.enums import (
-    ArchivePurpose,
     PackageDiffStatus,
     PackagePublishingStatus,
     )
+from lp.soyuz.interfaces.archive import MAIN_ARCHIVE_PURPOSES
 from lp.soyuz.interfaces.packagediff import IPackageDiffSet
 from lp.soyuz.interfaces.packageset import IPackagesetSet
 from lp.soyuz.model.archive import Archive
@@ -117,7 +117,10 @@
         )
     conditions = And(
         DistroSeriesDifference.id.is_in(dsd.id for dsd in dsds),
-        SourcePackagePublishingHistory.archiveID == Archive.id,
+        # The + 0 below prevents PostgreSQL from using the index named
+        # securesourcepackagepublishinghistory__archive__status__idx, the use
+        # of which results in a terrible query plan.
+        SourcePackagePublishingHistory.archiveID + 0 == Archive.id,
         SourcePackagePublishingHistory.sourcepackagereleaseID == (
             SourcePackageRelease.id),
         SourcePackagePublishingHistory.status.is_in(statuses),
@@ -129,16 +132,22 @@
         conditions = And(
             conditions,
             DistroSeries.id == DistroSeriesDifference.parent_series_id,
-            Archive.distributionID == DistroSeries.distributionID,
-            Archive.purpose == ArchivePurpose.PRIMARY,
+            SourcePackagePublishingHistory.distroseriesID == (
+                DistroSeriesDifference.parent_series_id),
             )
     else:
         conditions = And(
             conditions,
             DistroSeries.id == DistroSeriesDifference.derived_series_id,
-            Archive.distributionID == DistroSeries.distributionID,
-            Archive.purpose == ArchivePurpose.PRIMARY,
+            SourcePackagePublishingHistory.distroseriesID == (
+                DistroSeriesDifference.derived_series_id),
             )
+    # Ensure that the archive has the right purpose.
+    conditions = And(
+        conditions,
+        Archive.distributionID == DistroSeries.distributionID,
+        Archive.purpose.is_in(MAIN_ARCHIVE_PURPOSES),
+        )
     # Do we match on DistroSeriesDifference.(parent_)source_version?
     if match_version:
         if in_parent: