Merge lp:~julian-edwards/launchpad/slow-retry-depwait-bug-590757 into lp:launchpad

[Julian Edwards]

= Summary =
Fix slow retry-depwait script

== Proposed fix ==
buildd-retry-depwait was taking 10 minutes to run but since 10.05 it's taking
over 2 hours. This is due to some very inefficient queries which seem to have
been exposed as such by the recent build schema re-modelling.

== Pre-implementation notes ==
I've chatted to stub and there's a bunch of improvements made to the code and
to the queries:
 * Materialise query results early to avoid re-issuing long ones
 * Use the slave database where possible which will be much quicker than the
master, and also frees the master to make the whole of LP faster
 * The query in IArchive.findDepCandidateByName() was re-wrtitten in Storm.
It was using a view called PublishedPackage which was causing many unnecessary
joins on unrelated tables. The new query only joins what it has to, and it
also uses the slave store.

Unfortunately the use of a slave store means the tests needed a liberal
sprinkling of commit() which will slow them down. :(

There's also a bunch of lint fixed.

== Tests ==

bin/test -cvvt binarypackagebuild.txt -t test_binarypackagebuild

== Demo and Q/A ==

I've QA'ed this already on dogfood. Without the change it takes 9 hours to
run the script. With the change, it takes 50 minutes. I expect a similar
relative improvement on production.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/doc/archive.txt
  lib/lp/soyuz/interfaces/archive.py
  lib/lp/soyuz/tests/test_binarypackagebuild.py
  lib/lp/soyuz/model/archive.py
  lib/lp/soyuz/model/binarypackagebuild.py
  lib/lp/soyuz/doc/binarypackagebuild.txt

== Pylint notices ==

*Ridiculously wrong output deleted*

[Graham Binns (gmb)]

review: Approve (code)

[Francis J. Lacoste (flacoste)]

review: Approve (release-critical)

=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt	2010-05-19 15:39:52 +0000
+++ lib/lp/soyuz/doc/archive.txt	2010-06-08 11:45:47 +0000
@@ -1296,12 +1296,12 @@
 
     >>> candidate = ubuntu.main_archive.findDepCandidateByName(
     ...     warty_i386, "pmount")
-    >>> print candidate.binarypackagename
+    >>> print candidate.binarypackagerelease.binarypackagename.name
     pmount
 
     >>> candidate = cprov.archive.findDepCandidateByName(
     ...     warty_i386, "pmount")
-    >>> print candidate.binarypackagename
+    >>> print candidate.binarypackagerelease.binarypackagename.name
     pmount
 
 Since 'python2.4' isn't available in our sampledata (not even

=== modified file 'lib/lp/soyuz/doc/binarypackagebuild.txt'
--- lib/lp/soyuz/doc/binarypackagebuild.txt	2010-05-13 15:46:46 +0000
+++ lib/lp/soyuz/doc/binarypackagebuild.txt	2010-06-08 11:45:47 +0000
@@ -1004,10 +1004,13 @@
     >>> login('[e-mail]')
     >>> from lp.soyuz.interfaces.component import IComponentSet
     >>> main_component = getUtility(IComponentSet)['main']
+    >>> pmount_pub = hoary_i386[
+    ...     'pmount'].currentrelease.current_publishing_record
     >>> pmount_pub.component = main_component
     >>> depwait_build.dependencies = u'mozilla-firefox, pmount'
     >>> from canonical.database.sqlbase import flush_database_caches
     >>> flush_database_caches()
+    >>> transaction.commit()
     >>> login(ANONYMOUS)
 
     >>> flush_database_updates()

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2010-05-27 15:20:24 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2010-06-08 11:45:47 +0000
@@ -428,8 +428,8 @@
     def findDepCandidateByName(distroarchseries, name):
         """Return the last published binarypackage by given name.
 
-        Return the PublishedPackage record by binarypackagename or None if
-        not found.
+        Return the `BinaryPackagePublishingHistory` record by distroarchseries
+        and name, or None if not found.
         """
 
     def removeArchiveDependency(dependency):

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2010-06-04 15:00:40 +0000
+++ lib/lp/soyuz/model/archive.py	2010-06-08 11:45:47 +0000
@@ -15,7 +15,7 @@
 from sqlobject import  (
     BoolCol, ForeignKey, IntCol, StringCol)
 from sqlobject.sqlbuilder import SQLConstant
-from storm.expr import Or, And, Select, Sum
+from storm.expr import Or, And, Select, Sum, In, Desc
 from storm.locals import Count, Join
 from storm.store import Store
 from storm.zope.interfaces import IResultSet
@@ -32,6 +32,7 @@
 from canonical.database.enumcol import EnumCol
 from canonical.database.sqlbase import (
     cursor, quote, quote_like, sqlvalues, SQLBase)
+from canonical.launchpad.interfaces.lpstorm import ISlaveStore
 from lp.buildmaster.interfaces.buildbase import BuildStatus
 from lp.buildmaster.model.buildfarmjob import BuildFarmJob
 from lp.buildmaster.model.packagebuild import PackageBuild
@@ -42,6 +43,7 @@
 from lp.soyuz.model.archivedependency import ArchiveDependency
 from lp.soyuz.model.archiveauthtoken import ArchiveAuthToken
 from lp.soyuz.model.archivesubscriber import ArchiveSubscriber
+from lp.soyuz.model.binarypackagename import BinaryPackageName
 from lp.soyuz.model.binarypackagerelease import (
     BinaryPackageReleaseDownloadCount)
 from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
@@ -53,7 +55,6 @@
 from canonical.launchpad.database.librarian import (
     LibraryFileAlias, LibraryFileContent)
 from lp.soyuz.model.packagediff import PackageDiff
-from lp.soyuz.model.publishedpackage import PublishedPackage
 from lp.soyuz.model.publishing import (
     SourcePackagePublishingHistory, BinaryPackagePublishingHistory)
 from lp.soyuz.model.queue import PackageUpload, PackageUploadSource
@@ -801,15 +802,20 @@
             IResultSet(self.dependencies).values(
             ArchiveDependency.dependencyID))
 
-        query = """
-            binarypackagename = %s AND
-            distroarchseries = %s AND
-            archive IN %s AND
-            packagepublishingstatus = %s
-        """ % sqlvalues(name, distroarchseries, archives,
-                        PackagePublishingStatus.PUBLISHED)
-
-        return PublishedPackage.selectFirst(query, orderBy=['-id'])
+        store = ISlaveStore(BinaryPackagePublishingHistory)
+        candidate = store.find(
+            BinaryPackagePublishingHistory,
+            BinaryPackageName.name == name,
+            BinaryPackageRelease.binarypackagename == BinaryPackageName.id,
+            BinaryPackagePublishingHistory.binarypackagerelease ==
+                BinaryPackageRelease.id,
+            BinaryPackagePublishingHistory.distroarchseries ==
+                distroarchseries,
+            In(BinaryPackagePublishingHistory.archiveID, archives),
+            BinaryPackagePublishingHistory.status ==
+                PackagePublishingStatus.PUBLISHED
+            ).order_by(Desc(BinaryPackagePublishingHistory.id))
+        return candidate.first()
 
     def getArchiveDependency(self, dependency):
         """See `IArchive`."""
@@ -993,9 +999,10 @@
         distroseries = sourcepackage.distroseries
         sourcepackagename = sourcepackage.sourcepackagename
         component = sourcepackage.latest_published_component
-        # strict_component is True because the source package already exists
-        # (otherwise we couldn't have a suitesourcepackage object) and
-        # nascentupload passes True as a matter of policy when the package exists.
+        # strict_component is True because the source package already
+        # exists (otherwise we couldn't have a suitesourcepackage
+        # object) and nascentupload passes True as a matter of policy
+        # when the package exists.
         reason = self.checkUpload(
             person, distroseries, sourcepackagename, component, pocket,
             strict_component=True)
@@ -1019,7 +1026,7 @@
             if not distroseries.canUploadToPocket(pocket):
                 return CannotUploadToPocket(distroseries, pocket)
 
-    def _checkUpload(self, person, distroseries, sourcepackagename, component, 
+    def _checkUpload(self, person, distroseries, sourcepackagename, component,
                     pocket, strict_component=True):
         """See `IArchive`."""
         if isinstance(component, basestring):

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2010-05-21 09:42:21 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2010-06-08 11:45:47 +0000
@@ -33,6 +33,7 @@
      get_contact_email_addresses, get_email_template)
 from canonical.launchpad.interfaces.launchpad import (
     NotFoundError, ILaunchpadCelebrities)
+from canonical.launchpad.interfaces.lpstorm import IMasterObject, ISlaveStore
 from canonical.launchpad.mail import (
     simple_sendmail, format_address)
 from canonical.launchpad.webapp import canonical_url
@@ -380,7 +381,7 @@
             return False
 
         if not self._checkDependencyVersion(
-            dep_candidate.binarypackageversion, version, relation):
+            dep_candidate.binarypackagerelease.version, version, relation):
             return False
 
         # Only PRIMARY archive build dependencies should be restricted
@@ -391,7 +392,7 @@
         # only contains packages in 'main' component.
         ogre_components = get_components_for_building(self)
         if (self.archive.purpose == ArchivePurpose.PRIMARY and
-            dep_candidate.component not in ogre_components):
+            dep_candidate.component.name not in ogre_components):
             return False
 
         return True
@@ -542,7 +543,8 @@
         extra_headers = {
             'X-Launchpad-Build-State': self.status.name,
             'X-Launchpad-Build-Component' : self.current_component.name,
-            'X-Launchpad-Build-Arch' : self.distro_arch_series.architecturetag,
+            'X-Launchpad-Build-Arch' :
+                self.distro_arch_series.architecturetag,
             }
 
         # XXX cprov 2006-10-27: Temporary extra debug info about the
@@ -947,7 +949,7 @@
         logger = logging.getLogger('retry-depwait')
 
         # Get the MANUALDEPWAIT records for all archives.
-        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+        store = ISlaveStore(BinaryPackageBuild)
 
         candidates = store.find(
             BinaryPackageBuild,
@@ -956,7 +958,11 @@
             PackageBuild.build_farm_job == BuildFarmJob.id,
             BuildFarmJob.status == BuildStatus.MANUALDEPWAIT)
 
-        candidates_count = candidates.count()
+        # Materialise the results right away to avoid hitting the
+        # database multiple times.
+        candidates = list(candidates)
+
+        candidates_count = len(candidates)
         if candidates_count == 0:
             logger.info("No MANUALDEPWAIT record found.")
             return
@@ -967,6 +973,9 @@
         for build in candidates:
             if not build.can_be_retried:
                 continue
+            # We're changing 'build' so make sure we have an object from
+            # the master store.
+            build = IMasterObject(build)
             build.updateDependencies()
             if build.dependencies:
                 logger.debug(

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2010-05-11 21:52:52 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2010-06-08 11:45:47 +0000
@@ -161,6 +161,7 @@
         # Calling `IBinaryPackageBuild.updateDependencies` makes the build
         # record ready for dispatch.
         depwait_build = self._setupSimpleDepwaitContext()
+        self.layer.txn.commit()
         depwait_build.updateDependencies()
         self.assertEquals(depwait_build.dependencies, '')
 
@@ -203,6 +204,7 @@
         contrib = getUtility(IComponentSet).new('contrib')
         removeSecurityProxy(spr).component = contrib
 
+        self.layer.txn.commit()
         depwait_build.updateDependencies()
         self.assertEquals(depwait_build.dependencies, '')