Merge lp:~jtv/launchpad/bug-802335 into lp:launchpad

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 13318
Proposed branch: lp:~jtv/launchpad/bug-802335
Merge into: lp:launchpad
Diff against target: 48 lines (+10/-10)
1 file modified
lib/lp/soyuz/model/queue.py (+10/-10)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-802335
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+66148@code.launchpad.net

Commit message

[r=jtv][bug=802335] Performance problem with new PackageUpload name search.

Description of the change

= Summary =

We recently replaced the PackageUploads search logic on the DistroSeries:+queue page with a whole different code path, part of which was already available and part of which we had to develop for the purpose. The new code would find sync uploads as well as the more traditional types of upload.

The main search query in the new code path can time out when the search criteria include a name pattern. (The name pattern is applied to source package names, binary package names, and filenames for custom uploads).

== Proposed fix ==

The query is a fairly wide join, but not particularly complex. It left-joins various tables that might produce a matching name.

Those joins' conditions were sub-optimal. The join conditions did the name matching (which produces long scans and vast Cartesian products) after which the WHERE conditions established that the joined objects were actually related to the queried uploads.

Switching the two slightly complicates the ordering of the joins, but speeds things up by four orders of magnitude (roughly from a dozen seconds to a bit over a millisecond). No structural or functional changes are needed.

== Tests ==

{{{
./bin/test -vvc lp.soyuz.tests.test_queue -t getAll
}}}

== Demo and Q/A ==

Go to /ubuntu/oneiric/+queue and enter a short string in the name input field. In the example case it was "qt". If there are any uploads for matching source package names, binary package names, or custom files whose names contain the pattern, they will be shown. This includes sync uploads in particular.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/model/queue.py

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Julian says to self-review.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/model/queue.py'
2--- lib/lp/soyuz/model/queue.py 2011-06-24 09:41:05 +0000
3+++ lib/lp/soyuz/model/queue.py 2011-06-28 13:43:46 +0000
4@@ -1502,10 +1502,12 @@
5 if name is not None and name != '':
6 spn_join = LeftJoin(
7 SourcePackageName,
8- match_column(SourcePackageName.name, name))
9+ SourcePackageName.id ==
10+ SourcePackageRelease.sourcepackagenameID)
11 bpn_join = LeftJoin(
12 BinaryPackageName,
13- match_column(BinaryPackageName.name, name))
14+ BinaryPackageName.id ==
15+ BinaryPackageRelease.binarypackagenameID)
16 custom_join = LeftJoin(
17 PackageUploadCustom,
18 PackageUploadCustom.packageuploadID == PackageUpload.id)
19@@ -1516,12 +1518,12 @@
20
21 joins += [
22 package_copy_job_join,
23+ source_join,
24+ spr_join,
25 spn_join,
26- source_join,
27- spr_join,
28+ build_join,
29+ bpr_join,
30 bpn_join,
31- build_join,
32- bpr_join,
33 custom_join,
34 file_join,
35 ]
36@@ -1529,10 +1531,8 @@
37 # One of these attached items must have a matching name.
38 conditions.append(Or(
39 match_column(PackageCopyJob.package_name, name),
40- SourcePackageRelease.sourcepackagenameID ==
41- SourcePackageName.id,
42- BinaryPackageRelease.binarypackagenameID ==
43- BinaryPackageName.id,
44+ match_column(SourcePackageName.name, name),
45+ match_column(BinaryPackageName.name, name),
46 match_column(LibraryFileAlias.filename, name)))
47
48 if version is not None and version != '':