launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-802335 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #802335 in Launchpad itself: "DistroSeries:+queue timeouts when filtering by package name"
  https://bugs.launchpad.net/launchpad/+bug/802335

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-802335/+merge/66148

= 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
-- 
https://code.launchpad.net/~jtv/launchpad/bug-802335/+merge/66148
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-802335 into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2011-06-24 09:41:05 +0000
+++ lib/lp/soyuz/model/queue.py	2011-06-28 13:43:46 +0000
@@ -1502,10 +1502,12 @@
         if name is not None and name != '':
             spn_join = LeftJoin(
                 SourcePackageName,
-                match_column(SourcePackageName.name, name))
+                SourcePackageName.id ==
+                    SourcePackageRelease.sourcepackagenameID)
             bpn_join = LeftJoin(
                 BinaryPackageName,
-                match_column(BinaryPackageName.name, name))
+                BinaryPackageName.id ==
+                    BinaryPackageRelease.binarypackagenameID)
             custom_join = LeftJoin(
                 PackageUploadCustom,
                 PackageUploadCustom.packageuploadID == PackageUpload.id)
@@ -1516,12 +1518,12 @@
 
             joins += [
                 package_copy_job_join,
+                source_join,
+                spr_join,
                 spn_join,
-                source_join,
-                spr_join,
+                build_join,
+                bpr_join,
                 bpn_join,
-                build_join,
-                bpr_join,
                 custom_join,
                 file_join,
                 ]
@@ -1529,10 +1531,8 @@
             # One of these attached items must have a matching name.
             conditions.append(Or(
                 match_column(PackageCopyJob.package_name, name),
-                SourcePackageRelease.sourcepackagenameID ==
-                    SourcePackageName.id,
-                BinaryPackageRelease.binarypackagenameID ==
-                    BinaryPackageName.id,
+                match_column(SourcePackageName.name, name),
+                match_column(BinaryPackageName.name, name),
                 match_column(LibraryFileAlias.filename, name)))
 
         if version is not None and version != '':