launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-241129-queue-binary-scaling into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #241129 'queue override' command scales at O(n^2) with the number of packages on the commandline
  https://bugs.launchpad.net/bugs/241129


The 'queue' archive admin commandline tool lets users specify multiple binary names to override in a single invocation.

However, the current implementation fetches the PackageUpload matching each binary, and overrides them all without removing duplicates. This is inefficient when overriding multiple binaries from the same build: each binary is overridden n² times (bug #241129)!

This branch fixes the scaling problem by only considering each PackageUpload once. It also adds tests to confirm the number of overridden packages does not increase.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-241129-queue-binary-scaling/+merge/31604
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-241129-queue-binary-scaling into lp:launchpad/devel.

=== modified file 'lib/lp/soyuz/scripts/queue.py'
--- lib/lp/soyuz/scripts/queue.py	2010-08-02 02:13:52 +0000
+++ lib/lp/soyuz/scripts/queue.py	2010-08-03 04:16:03 +0000
@@ -189,7 +189,8 @@
                            self.distroseries.distribution.name,
                            self.distroseries.name, self.pocket.name))
 
-                self.items.append(item)
+                if item not in self.items:
+                    self.items.append(item)
                 self.explicit_ids_specified = True
             else:
                 # retrieve PackageUpload item by name/version key
@@ -201,7 +202,8 @@
                 for item in self.distroseries.getQueueItems(
                     status=self.queue, name=term, version=version,
                     exact_match=self.exact_match, pocket=self.pocket):
-                    self.items.append(item)
+                    if item not in self.items:
+                        self.items.append(item)
                 self.package_names.append(term)
 
         self.items_size = len(self.items)
@@ -529,6 +531,7 @@
                              priority_name, announcelist, display,
                              no_mail=True, exact_match=False, log=log)
         self.terms_start_index = 1
+        self.overrides_performed = 0
 
     def run(self):
         """Perform Override action."""
@@ -575,6 +578,7 @@
                         component,
                         queue_item.sourcepackagerelease.component
                         ])
+                self.overrides_performed += 1
             self.displayInfo(queue_item)
 
     def _override_binary(self):
@@ -614,6 +618,7 @@
                                         binary.priority.name))
                         binary.override(component=component, section=section,
                                         priority=priority)
+                        self.overrides_performed += 1
                         self.displayInfo(queue_item, only=binary.name)
                 # See if the new component requires a new archive on the
                 # build:

=== modified file 'lib/lp/soyuz/scripts/tests/test_queue.py'
--- lib/lp/soyuz/scripts/tests/test_queue.py	2010-02-09 00:17:40 +0000
+++ lib/lp/soyuz/scripts/tests/test_queue.py	2010-08-03 04:16:03 +0000
@@ -719,6 +719,7 @@
             component_name='universe', section_name='editors')
         # 'netapplet' appears 3 times, alsa-utils once.
         self.assertEqual(4, queue_action.items_size)
+        self.assertEqual(2, queue_action.overrides_performed)
         # Check results.
         queue_items = list(breezy_autotest.getQueueItems(
             status=PackageUploadStatus.NEW, name='alsa-utils'))
@@ -814,6 +815,7 @@
             priority_name='optional')
         # Check results.
         self.assertEqual(2, queue_action.items_size)
+        self.assertEqual(2, queue_action.overrides_performed)
         queue_items = list(breezy_autotest.getQueueItems(
             status=PackageUploadStatus.NEW, name='pmount'))
         queue_items.extend(list(breezy_autotest.getQueueItems(
@@ -861,7 +863,13 @@
             component_name='restricted', section_name='editors',
             priority_name='optional')
 
-        self.assertEqual(2, queue_action.items_size)
+        # There are three binaries to override on this PackageUpload:
+        #  - mozilla-firefox in breezy-autotest
+        #  - mozilla-firefox and mozilla-firefox-data in warty
+        # Each should be overridden exactly once.
+        self.assertEqual(1, queue_action.items_size)
+        self.assertEqual(3, queue_action.overrides_performed)
+
         queue_items = list(breezy_autotest.getQueueItems(
             status=PackageUploadStatus.NEW, name='mozilla-firefox-data'))
         queue_items.extend(list(breezy_autotest.getQueueItems(