launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01782
lp:~edwin-grubbs/launchpad/bug-668047-project-dsp-portlet-timeout into lp:launchpad/devel
Edwin Grubbs has proposed merging lp:~edwin-grubbs/launchpad/bug-668047-project-dsp-portlet-timeout into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#668047 Project:+index : Project page timeout on qastaging with 11s of SQL
https://bugs.launchpad.net/bugs/668047
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.
--
https://code.launchpad.net/~edwin-grubbs/launchpad/bug-668047-project-dsp-portlet-timeout/+merge/39917
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~edwin-grubbs/launchpad/bug-668047-project-dsp-portlet-timeout into lp:launchpad/devel.
=== 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-03 02:02:46 +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-02 06:21:58 +0000
+++ lib/lp/registry/model/distribution.py 2010-11-03 02:02:46 +0000
@@ -1164,16 +1164,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
@@ -1200,8 +1201,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-03 02:02:46 +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',
