Merge lp:~sinzui/launchpad/ds-getcurrentreleases into lp:launchpad

Proposed by Curtis Hovey on 2010-11-12
Status: Merged
Approved by: Curtis Hovey on 2010-11-12
Approved revision: no longer in the source branch.
Merge reported by: Curtis Hovey
Merged at revision: not available
Proposed branch: lp:~sinzui/launchpad/ds-getcurrentreleases
Merge into: lp:launchpad
Diff against target: 42 lines (+13/-12)
1 file modified
lib/lp/registry/model/distroseries.py (+13/-12)
To merge this branch: bzr merge lp:~sinzui/launchpad/ds-getcurrentreleases
Reviewer Review Type Date Requested Status
Henning Eggers (community) release-critical Approve on 2010-11-12
Edwin Grubbs (community) code 2010-11-12 Approve on 2010-11-12
Review via email: mp+40756@code.launchpad.net

Description of the Change

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.

To post a comment you must log in.
Edwin Grubbs (edwin-grubbs) wrote :

Looks good.

review: Approve (code)
Henning Eggers (henninge) wrote :

Thanks for figuring this out.

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/model/distroseries.py'
2--- lib/lp/registry/model/distroseries.py 2010-11-12 06:38:36 +0000
3+++ lib/lp/registry/model/distroseries.py 2010-11-12 20:21:26 +0000
4@@ -983,25 +983,26 @@
5 """See `IDistroSeries`."""
6 source_package_ids = [
7 package_name.id for package_name in source_package_names]
8- releases = SourcePackageRelease.select("""
9- SourcePackageRelease.sourcepackagename IN %s AND
10- SourcePackageRelease.id =
11- SourcePackagePublishingHistory.sourcepackagerelease AND
12- SourcePackagePublishingHistory.id = (
13- SELECT max(spph.id)
14+ # Construct a table of just current releases for the desired SPNs.
15+ origin = SQL("""
16+ SourcePackageRelease
17+ JOIN (
18+ SELECT
19+ spr.id as sourcepackagerelease, MAX(spph.id)
20 FROM SourcePackagePublishingHistory spph,
21- SourcePackageRelease spr
22+ SourcePackageRelease spr
23 WHERE
24- spr.sourcepackagename = SourcePackageRelease.sourcepackagename AND
25+ spr.sourcepackagename IN %s AND
26 spph.sourcepackagerelease = spr.id AND
27 spph.archive IN %s AND
28 spph.status IN %s AND
29- spph.distroseries = %s)
30+ spph.distroseries = %s
31+ GROUP BY spr.id)
32+ AS spph ON SourcePackageRelease.id = spph.sourcepackagerelease
33 """ % sqlvalues(
34 source_package_ids, self.distribution.all_distro_archive_ids,
35- active_publishing_status, self),
36- clauseTables=[
37- 'SourcePackagePublishingHistory'])
38+ active_publishing_status, self))
39+ releases = IStore(self).using(origin).find(SourcePackageRelease)
40 return dict(
41 (self.getSourcePackage(release.sourcepackagename),
42 DistroSeriesSourcePackageRelease(self, release))