Merge lp:~edwin-grubbs/launchpad/bug-668047-project-dsp-portlet-timeout into lp:launchpad

[Edwin Grubbs]

Fix timeout finding suggested ubuntu packages to link to a project.

Summary
-------

The project page would timeout when the "Packages in Ubuntu" portlet
would find a huge number of packages matching the project's name that
could be suggested for linking. The subquery for checking the
SourcePackageRelease and SourcePackagePublishingHistory table forced
postgres to use a nested loop, which was very inefficent.

Switching to using a join made it about 200 times faster. Because there
can be multiple SPPH records per DSP, the SELECT had to be made
DISTINCT. The DISTINCT caused some of the tests to break because the
order of the results was nondeterministic when the rank is NULL (i.e.
when a DSP is matched with LIKE as opposed to with the fti). I added the
DSP.name to the ORDER BY clause to make the tests less fragile.

Tests
-----

./bin/test -vv -t 'doc/distribution.txt|xx-distribution.txt'

Demo and Q/A
------------

* Open https://qastaging.launchpad.net/do
  * It should not timeout.

[Robert Collins (lifeless)]

Nice.

review: Approve

=== modified file 'lib/lp/registry/browser/tests/product-portlet-packages-view.txt'
--- lib/lp/registry/browser/tests/product-portlet-packages-view.txt	2010-10-19 18:44:31 +0000
+++ lib/lp/registry/browser/tests/product-portlet-packages-view.txt	2010-11-12 16:05:06 +0000
@@ -179,8 +179,8 @@
     ...     principal=product.owner)
     >>> for dsp in view.suggestions:
     ...     print dsp.name
+    ba-bingo
     bingo
-    ba-bingo
 
     >>> content = find_tag_by_id(view.render(), 'portlet-packages')
     >>> print extract_text(content)
@@ -191,8 +191,8 @@
     let distribution and upstream maintainers share bugs, patches, and
     translations efficiently.
     Ubuntu Hoary packages:
-    bingo
-    ba-bingo...
+    ba-bingo
+    bingo...
 
 But the limit is 8 packages.
 

=== modified file 'lib/lp/registry/doc/distribution.txt'
--- lib/lp/registry/doc/distribution.txt	2010-11-02 05:48:54 +0000
+++ lib/lp/registry/doc/distribution.txt	2010-11-12 16:05:06 +0000
@@ -172,8 +172,11 @@
 a certain string through the magic of fti. For instance:
 
     >>> packages = ubuntu.searchSourcePackageCaches("mozilla")
-    >>> print packages.count()
-    1
+    >>> for distro_source_package_cache, source_name, rank in packages:
+    ...     print "%-17s rank:%s" % (
+    ...         distro_source_package_cache.name,
+    ...         type(rank))
+    mozilla-firefox   rank:<type 'float'>
 
 The search also matches on exact package names which fti doesn't like,
 and even on substrings:
@@ -183,14 +186,15 @@
     1
     >>> packages = ubuntu.searchSourcePackageCaches('a')
     >>> for distro_source_package_cache, source_name, rank in packages:
-    ...     print "%s: %s" % (
+    ...     print "%s: %-17s rank:%s" % (
     ...         distro_source_package_cache.__class__.__name__,
-    ...         distro_source_package_cache.name)
-    DistributionSourcePackageCache: mozilla-firefox
-    DistributionSourcePackageCache: netapplet
-    DistributionSourcePackageCache: alsa-utils
-    DistributionSourcePackageCache: foobar
-    DistributionSourcePackageCache: commercialpackage
+    ...         distro_source_package_cache.name,
+    ...         type(rank))
+    DistributionSourcePackageCache: alsa-utils        rank:<type 'NoneType'>
+    DistributionSourcePackageCache: commercialpackage rank:<type 'NoneType'>
+    DistributionSourcePackageCache: foobar            rank:<type 'NoneType'>
+    DistributionSourcePackageCache: mozilla-firefox   rank:<type 'NoneType'>
+    DistributionSourcePackageCache: netapplet         rank:<type 'NoneType'>
 
 The searchSourcePackages() method just returns a decorated version
 of the results from searchSourcePackageCaches():
@@ -198,11 +202,11 @@
     >>> packages = ubuntu.searchSourcePackages('a')
     >>> for dsp in packages:
     ...     print "%s: %s" % (dsp.__class__.__name__, dsp.name)
+    DistributionSourcePackage: alsa-utils
+    DistributionSourcePackage: commercialpackage
+    DistributionSourcePackage: foobar
     DistributionSourcePackage: mozilla-firefox
     DistributionSourcePackage: netapplet
-    DistributionSourcePackage: alsa-utils
-    DistributionSourcePackage: foobar
-    DistributionSourcePackage: commercialpackage
 
 searchSourcePackages() also has a has_packaging parameter that
 it just passes on to searchSourcePackageCaches(), and it restricts
@@ -212,14 +216,14 @@
     >>> packages = ubuntu.searchSourcePackages('a', has_packaging=True)
     >>> for dsp in packages:
     ...     print "%s: %s" % (dsp.__class__.__name__, dsp.name)
+    DistributionSourcePackage: alsa-utils
     DistributionSourcePackage: mozilla-firefox
     DistributionSourcePackage: netapplet
-    DistributionSourcePackage: alsa-utils
     >>> packages = ubuntu.searchSourcePackages('a', has_packaging=False)
     >>> for dsp in packages:
     ...     print "%s: %s" % (dsp.__class__.__name__, dsp.name)
+    DistributionSourcePackage: commercialpackage
     DistributionSourcePackage: foobar
-    DistributionSourcePackage: commercialpackage
 
 searchSourcePackages() also has a publishing_distroseries parameter that
 it just passes on to searchSourcePackageCaches(), and it restricts the
@@ -230,8 +234,8 @@
     ...     'a', publishing_distroseries=ubuntu.currentseries)
     >>> for dsp in packages:
     ...     print "%s: %s" % (dsp.__class__.__name__, dsp.name)
+    DistributionSourcePackage: alsa-utils
     DistributionSourcePackage: netapplet
-    DistributionSourcePackage: alsa-utils
 
 
 Searching for binary packages

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2010-11-05 14:56:34 +0000
+++ lib/lp/registry/model/distribution.py	2010-11-12 16:05:06 +0000
@@ -1142,16 +1142,17 @@
 
         publishing_condition = ''
         if publishing_distroseries is not None:
-            publishing_condition = """
-                AND EXISTS (
-                    SELECT 1
-                    FROM SourcePackageRelease spr
-                        JOIN SourcePackagePublishingHistory spph
-                            ON spph.sourcepackagerelease = spr.id
-                    WHERE spr.sourcepackagename = SourcePackageName.id
-                        AND spph.distroseries = %d
-                    )""" % (
-                publishing_distroseries.id)
+            origin += [
+                Join(SourcePackageRelease,
+                    SourcePackageRelease.sourcepackagename ==
+                        SourcePackageName.id),
+                Join(SourcePackagePublishingHistory,
+                    SourcePackagePublishingHistory.sourcepackagerelease ==
+                        SourcePackageRelease.id),
+                ]
+            publishing_condition = (
+                "AND SourcePackagePublishingHistory.distroseries = %d"
+                % publishing_distroseries.id)
 
         packaging_query = """
             SELECT 1
@@ -1178,8 +1179,9 @@
                    quote(text), quote_like(text), has_packaging_condition,
                    publishing_condition)
         dsp_caches_with_ranks = store.using(*origin).find(
-            find_spec, condition).order_by('rank DESC')
-
+            find_spec, condition).order_by(
+                'rank DESC, DistributionSourcePackageCache.name')
+        dsp_caches_with_ranks.config(distinct=True)
         return dsp_caches_with_ranks
 
     def searchSourcePackages(

=== modified file 'lib/lp/registry/stories/webservice/xx-distribution.txt'
--- lib/lp/registry/stories/webservice/xx-distribution.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/registry/stories/webservice/xx-distribution.txt	2010-11-12 16:05:06 +0000
@@ -1,4 +1,5 @@
-= Distributions =
+Distributions
+=============
 
 At the top level we provide the collection of all distributions, with
 Ubuntu and its flavours being the first on the list.
@@ -50,7 +51,8 @@
     title: u'Ubuntu Linux'
 
 
-== Distribution Custom Operations ==
+Distribution Custom Operations
+==============================
 
 Distribution has some custom operations.
 
@@ -113,11 +115,11 @@
 
     >>> for entry in alsa_results['entries']:
     ...     print entry['self_link']
+    http://.../ubuntu/+source/alsa-utils
+    http://.../ubuntu/+source/commercialpackage
+    http://.../ubuntu/+source/foobar
     http://.../ubuntu/+source/mozilla-firefox
     http://.../ubuntu/+source/netapplet
-    http://.../ubuntu/+source/alsa-utils
-    http://.../ubuntu/+source/foobar
-    http://.../ubuntu/+source/commercialpackage
 
 "getArchive" returns a distribution archive (not a PPA) with the given name.
 
@@ -190,7 +192,6 @@
     >>> antarctica_patch_target = webservice.named_get(
     ...     ubuntu['self_link'], 'getMirrorByName',
     ...     name='mirror.showa.antarctica.org-archive').jsonBody()
-    ... )
 
     >>> response = karl_webservice.patch(
     ...     antarctica_patch_target['self_link'], 'application/json',

=== modified file 'lib/lp/soyuz/browser/tests/distribution-views.txt'
--- lib/lp/soyuz/browser/tests/distribution-views.txt	2010-06-28 15:22:25 +0000
+++ lib/lp/soyuz/browser/tests/distribution-views.txt	2010-11-12 16:05:06 +0000
@@ -94,11 +94,11 @@
     False
     >>> for package in distro_pkg_search_view.search_results:
     ...     print package.name
-    netapplet
     alsa-utils
+    commercialpackage
     foobar
     mozilla-firefox
-    commercialpackage
+    netapplet
 
 Unless the distribution being searched does not support binaries, in which
 cases it will always be on source: