launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/ds-getcurrentreleases into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #674672 SourcePackage:+index timeout at distroseries.getCurrentReleases
  https://bugs.launchpad.net/bugs/674672


This is my branch to make distroseries.getCurrentReleases faster.

    lp:~sinzui/launchpad/devel
    Diff size: 43
    Launchpad bug:
        https://bugs.launchpad.net/bugs/674672
    Test command: ./bin/test -vv \
        -t TestDistroSeriesCurrentSourceReleases \
        -t distributionsourcepackage-views -t distroseries-views \
        -t packaging-views -t test_packaging -t test_sourcepackage_views \
        -t "registry/.*/stories/(distroseries|packaging)"
    Pre-implementation: EdwinGrubbs
    Target release: 10.11


Make distroseries.getCurrentReleases faster
-------------------------------------------

The update the PG 8.4 caused several pages like +packages and +source to
become slower. The root cause is that distroseries.getCurrentReleases()
is working with more SPRs in the query than we intend. For example, when
getting the current release of bzr in natty, 1000+ releases are in the working
set of data.


Rules
-----

    * Conver the subquery in the WHERE clause to be a query table that
      has exactly 1 for each SPN being looked up.
    * Stormify the query.


QA
--

    * Visit https://qastaging.launchapd.net/ubuntu/natty/+source/bzr
    * Verify the page loads.
    * Visit https://qastaging.launchapd.net/ubuntu/lucid/+source/bzr
    * Verify the page loads.
    * Visit https://launchpad.net/ubuntu/natty/+packaging
    * Verify the page loads.
    * Visit https://qastaging.launchpad.net/~yavdr/+archive/stable-vdr/+packages
    * Verify the page loads.

Lint
----

Linting changed files:
  lib/lp/registry/model/distroseries.py


Test
----

No tests changed, but I certainly ran a lot to verify the output did not
change. The query was tested against the staging db

http://pastebin.ubuntu.com/530801/ 9.6 seconds before the fix.
http://pastebin.ubuntu.com/530817/ 0.0017 seconds after the fix.
-- 
https://code.launchpad.net/~sinzui/launchpad/ds-getcurrentreleases/+merge/40756
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/ds-getcurrentreleases into lp:launchpad/devel.

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2010-11-12 06:38:36 +0000
+++ lib/lp/registry/model/distroseries.py	2010-11-12 20:21:26 +0000
@@ -983,25 +983,26 @@
         """See `IDistroSeries`."""
         source_package_ids = [
             package_name.id for package_name in source_package_names]
-        releases = SourcePackageRelease.select("""
-            SourcePackageRelease.sourcepackagename IN %s AND
-            SourcePackageRelease.id =
-                SourcePackagePublishingHistory.sourcepackagerelease AND
-            SourcePackagePublishingHistory.id = (
-                SELECT max(spph.id)
+        # Construct a table of just current releases for the desired SPNs.
+        origin = SQL("""
+            SourcePackageRelease
+            JOIN (
+                SELECT
+                    spr.id as sourcepackagerelease, MAX(spph.id)
                 FROM SourcePackagePublishingHistory spph,
-                     SourcePackageRelease spr
+                    SourcePackageRelease spr
                 WHERE
-                    spr.sourcepackagename = SourcePackageRelease.sourcepackagename AND
+                    spr.sourcepackagename IN %s AND
                     spph.sourcepackagerelease = spr.id AND
                     spph.archive IN %s AND
                     spph.status IN %s AND
-                    spph.distroseries = %s)
+                    spph.distroseries = %s
+                GROUP BY spr.id)
+                AS spph ON SourcePackageRelease.id = spph.sourcepackagerelease
             """ % sqlvalues(
                 source_package_ids, self.distribution.all_distro_archive_ids,
-                active_publishing_status, self),
-            clauseTables=[
-                'SourcePackagePublishingHistory'])
+                active_publishing_status, self))
+        releases = IStore(self).using(origin).find(SourcePackageRelease)
         return dict(
             (self.getSourcePackage(release.sourcepackagename),
              DistroSeriesSourcePackageRelease(self, release))