Merge lp:~wgrant/launchpad/die-spr-spn into lp:launchpad

[William Grant]

Port lots of queries to SourcePackagePublishingHistory.sourcepackagename for speed.

This branch enables more widespread use of the relatively new denormalised SourcePackagePublishingHistory.sourcepackagename column. Many queries want to find a SourcePackagePublishingHistory for a particular name in a given archive and distroseries -- this column and its indices let that be satisfied very quickly, rather than having to slowly join 25000 rows through SourcePackageRelease.

I grepped for SourcePackageRelease.sourcepackagename and replaced it with SPPH.spn in queries that primarily filter based on SPPH. In many cases the SPR join goes away entirely; in others it just becomes a secondary filter or sort. A few cases were left untouched because it wasn't an obvious substantial improvement; they'll need case-by-case analysis later.

[Steve Kowalik (stevenk)]

This looks excellent.

review: Approve (code)

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2012-05-01 10:20:07 +0000
+++ lib/lp/registry/model/distribution.py	2012-05-14 05:23:21 +0000
@@ -1249,9 +1249,8 @@
                 # timeouts).
                 SourcePackagePublishingHistory.archiveID.is_in(
                     self.all_distro_archive_ids),
-                SourcePackagePublishingHistory.sourcepackagereleaseID ==
-                    SourcePackageRelease.id,
-                SourcePackageRelease.sourcepackagename == sourcepackagename,
+                SourcePackagePublishingHistory.sourcepackagename ==
+                    sourcepackagename,
                 SourcePackagePublishingHistory.status.is_in(
                     (PackagePublishingStatus.PUBLISHED,
                      PackagePublishingStatus.PENDING)

=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py	2012-02-08 06:00:46 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py	2012-05-14 05:23:21 +0000
@@ -105,9 +105,7 @@
                 SourcePackagePublishingHistory.distroseriesID ==
                     DistroSeries.id,
                 DistroSeries.distributionID == obj.distribution.id,
-                SourcePackagePublishingHistory.sourcepackagereleaseID ==
-                    SourcePackageRelease.id,
-                SourcePackageRelease.sourcepackagenameID ==
+                SourcePackagePublishingHistory.sourcepackagenameID ==
                     obj.sourcepackagename.id).order_by(
                         Desc(SourcePackagePublishingHistory.id)).first()
             obj._new(obj.distribution, obj.sourcepackagename,
@@ -233,9 +231,7 @@
         spph = SourcePackagePublishingHistory.selectFirst("""
             SourcePackagePublishingHistory.distroseries = DistroSeries.id AND
             DistroSeries.distribution = %s AND
-            SourcePackagePublishingHistory.sourcepackagerelease =
-                SourcePackageRelease.id AND
-            SourcePackageRelease.sourcepackagename = %s AND
+            SourcePackagePublishingHistory.sourcepackagename = %s AND
             SourcePackagePublishingHistory.archive IN %s AND
             pocket NOT IN (%s, %s) AND
             status in (%s, %s)""" %
@@ -401,9 +397,9 @@
             SourcePackagePublishingHistory.archive IN %s AND
             SourcePackagePublishingHistory.distroseries =
                 DistroSeries.id AND
-            SourcePackagePublishingHistory.sourcepackagerelease =
-                SourcePackageRelease.id AND
-            SourcePackageRelease.sourcepackagename = %s
+            SourcePackagePublishingHistory.sourcepackagename = %s AND
+            SourcePackageRelease.id =
+                SourcePackagePublishingHistory.sourcepackagerelease
             """ % sqlvalues(self.distribution,
                             self.distribution.all_distro_archive_ids,
                             self.sourcepackagename)
@@ -426,9 +422,10 @@
             DistroSeries.distribution == self.distribution,
             SourcePackagePublishingHistory.archiveID.is_in(
                self.distribution.all_distro_archive_ids),
-            SourcePackagePublishingHistory.sourcepackagerelease ==
-                SourcePackageRelease.id,
-            SourcePackageRelease.sourcepackagename == self.sourcepackagename)
+            SourcePackagePublishingHistory.sourcepackagename ==
+                self.sourcepackagename,
+            SourcePackageRelease.id ==
+                SourcePackagePublishingHistory.sourcepackagereleaseID)
         result.order_by(
             Desc(SourcePackageRelease.id),
             Desc(SourcePackagePublishingHistory.datecreated),

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2012-04-05 14:39:45 +0000
+++ lib/lp/registry/model/distroseries.py	2012-05-14 05:23:21 +0000
@@ -738,49 +738,27 @@
 
     def updatePackageCount(self):
         """See `IDistroSeries`."""
-
-        # first update the source package count
-        query = """
-            SourcePackagePublishingHistory.distroseries = %s AND
-            SourcePackagePublishingHistory.archive IN %s AND
-            SourcePackagePublishingHistory.status IN %s AND
-            SourcePackagePublishingHistory.pocket = %s AND
-            SourcePackagePublishingHistory.sourcepackagerelease =
-                SourcePackageRelease.id AND
-            SourcePackageRelease.sourcepackagename =
-                SourcePackageName.id
-            """ % sqlvalues(
-                    self,
-                    self.distribution.all_distro_archive_ids,
-                    active_publishing_status,
-                    PackagePublishingPocket.RELEASE)
-        self.sourcecount = SourcePackageName.select(
-            query, distinct=True,
-            clauseTables=['SourcePackageRelease',
-                          'SourcePackagePublishingHistory']).count()
-
-        # next update the binary count
-        clauseTables = ['DistroArchSeries', 'BinaryPackagePublishingHistory',
-                        'BinaryPackageRelease']
-        query = """
-            BinaryPackagePublishingHistory.binarypackagerelease =
-                BinaryPackageRelease.id AND
-            BinaryPackageRelease.binarypackagename =
-                BinaryPackageName.id AND
-            BinaryPackagePublishingHistory.status IN %s AND
-            BinaryPackagePublishingHistory.pocket = %s AND
-            BinaryPackagePublishingHistory.distroarchseries =
-                DistroArchSeries.id AND
-            DistroArchSeries.distroseries = %s AND
-            BinaryPackagePublishingHistory.archive IN %s
-            """ % sqlvalues(
-                    active_publishing_status,
-                    PackagePublishingPocket.RELEASE,
-                    self,
-                    self.distribution.all_distro_archive_ids)
-        ret = BinaryPackageName.select(
-            query, distinct=True, clauseTables=clauseTables).count()
-        self.binarycount = ret
+        self.sourcecount = IStore(SourcePackagePublishingHistory).find(
+            SourcePackagePublishingHistory.sourcepackagenameID,
+            SourcePackagePublishingHistory.distroseries == self,
+            SourcePackagePublishingHistory.archiveID.is_in(
+                self.distribution.all_distro_archive_ids),
+            SourcePackagePublishingHistory.status.is_in(
+                active_publishing_status),
+            SourcePackagePublishingHistory.pocket ==
+                PackagePublishingPocket.RELEASE).config(distinct=True).count()
+
+        self.binarycount = IStore(BinaryPackagePublishingHistory).find(
+            BinaryPackagePublishingHistory.binarypackagenameID,
+            DistroArchSeries.distroseries == self,
+            BinaryPackagePublishingHistory.distroarchseriesID ==
+                DistroArchSeries.id,
+            BinaryPackagePublishingHistory.archiveID.is_in(
+                self.distribution.all_distro_archive_ids),
+            BinaryPackagePublishingHistory.status.is_in(
+                active_publishing_status),
+            BinaryPackagePublishingHistory.pocket ==
+                PackagePublishingPocket.RELEASE).config(distinct=True).count()
 
     @property
     def architecturecount(self):
@@ -1201,9 +1179,7 @@
         archives = self.distribution.getArchiveIDList(archive)
 
         clause = """
-            SourcePackagePublishingHistory.sourcepackagerelease=
-                SourcePackageRelease.id AND
-            SourcePackageRelease.sourcepackagename=
+            SourcePackagePublishingHistory.sourcepackagename=
                 SourcePackageName.id AND
             SourcePackagePublishingHistory.distroseries=%s AND
             SourcePackagePublishingHistory.archive IN %s AND
@@ -1217,7 +1193,7 @@
                 % sqlvalues(component))
 
         orderBy = ['SourcePackageName.name']
-        clauseTables = ['SourcePackageRelease', 'SourcePackageName']
+        clauseTables = ['SourcePackageName']
 
         return SourcePackagePublishingHistory.select(
             clause, orderBy=orderBy, clauseTables=clauseTables)

=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py	2012-02-07 07:37:27 +0000
+++ lib/lp/registry/model/sourcepackage.py	2012-05-14 05:23:21 +0000
@@ -17,7 +17,7 @@
 from storm.locals import (
     And,
     Desc,
-    Select,
+    Join,
     Store,
     )
 from zope.component import getUtility
@@ -245,7 +245,7 @@
         clauses.append(
                 """SourcePackagePublishingHistory.sourcepackagerelease =
                    SourcePackageRelease.id AND
-                   SourcePackageRelease.sourcepackagename = %s AND
+                   SourcePackagePublishingHistory.sourcepackagename = %s AND
                    SourcePackagePublishingHistory.distroseries = %s AND
                    SourcePackagePublishingHistory.archive IN %s
                 """ % sqlvalues(
@@ -380,21 +380,22 @@
 
         The results are ordered by descending version.
         """
-        subselect = Select(
-            SourcePackageRelease.id, And(
+        return IStore(SourcePackageRelease).using(
+            SourcePackageRelease,
+            Join(
+                SourcePackagePublishingHistory,
+                SourcePackagePublishingHistory.sourcepackagereleaseID ==
+                    SourcePackageRelease.id)
+            ).find(
+                SourcePackageRelease,
+                SourcePackagePublishingHistory.archiveID.is_in(
+                    self.distribution.all_distro_archive_ids),
                 SourcePackagePublishingHistory.distroseries ==
                     self.distroseries,
-                SourcePackagePublishingHistory.sourcepackagereleaseID ==
-                    SourcePackageRelease.id,
-                SourcePackageRelease.sourcepackagename ==
-                    self.sourcepackagename,
-                SourcePackagePublishingHistory.archiveID.is_in(
-                    self.distribution.all_distro_archive_ids)))
-
-        return IStore(SourcePackageRelease).find(
-            SourcePackageRelease,
-            SourcePackageRelease.id.is_in(subselect)).order_by(Desc(
-                SourcePackageRelease.version))
+                SourcePackagePublishingHistory.sourcepackagename ==
+                    self.sourcepackagename
+            ).config(distinct=True).order_by(
+                Desc(SourcePackageRelease.version))
 
     @property
     def name(self):
@@ -644,7 +645,7 @@
         condition_clauses = ["""
         BinaryPackageBuild.source_package_release =
             SourcePackageRelease.id AND
-        SourcePackageRelease.sourcepackagename = %s AND
+        SourcePackagePublishingHistory.sourcepackagename = %s AND
         SourcePackagePublishingHistory.distroseries = %s AND
         SourcePackagePublishingHistory.archive IN %s AND
         SourcePackagePublishingHistory.sourcepackagerelease =

=== modified file 'lib/lp/soyuz/adapters/overrides.py'
--- lib/lp/soyuz/adapters/overrides.py	2012-01-06 11:08:30 +0000
+++ lib/lp/soyuz/adapters/overrides.py	2012-05-14 05:23:21 +0000
@@ -191,7 +191,6 @@
     def calculateSourceOverrides(self, archive, distroseries, pocket, spns,
                                  source_component=None):
         # Avoid circular imports.
-        from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
         from lp.soyuz.model.publishing import SourcePackagePublishingHistory
 
         def eager_load(rows):
@@ -201,7 +200,7 @@
         store = IStore(SourcePackagePublishingHistory)
         already_published = DecoratedResultSet(
             store.find(
-                (SourcePackageRelease.sourcepackagenameID,
+                (SourcePackagePublishingHistory.sourcepackagenameID,
                  SourcePackagePublishingHistory.componentID,
                  SourcePackagePublishingHistory.sectionID),
                 SourcePackagePublishingHistory.archiveID == archive.id,
@@ -209,15 +208,14 @@
                     distroseries.id,
                 SourcePackagePublishingHistory.status.is_in(
                     active_publishing_status),
-                SourcePackageRelease.id ==
-                    SourcePackagePublishingHistory.sourcepackagereleaseID,
-                SourcePackageRelease.sourcepackagenameID.is_in(
+                SourcePackagePublishingHistory.sourcepackagenameID.is_in(
                     spn.id for spn in spns)).order_by(
-                        SourcePackageRelease.sourcepackagenameID,
+                        SourcePackagePublishingHistory.sourcepackagenameID,
                         Desc(SourcePackagePublishingHistory.datecreated),
                         Desc(SourcePackagePublishingHistory.id),
                 ).config(
-                    distinct=(SourcePackageRelease.sourcepackagenameID,)),
+                    distinct=(
+                        SourcePackagePublishingHistory.sourcepackagenameID,)),
             id_resolver((SourcePackageName, Component, Section)),
             pre_iter_hook=eager_load)
         return [

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2012-05-04 12:02:44 +0000
+++ lib/lp/soyuz/model/archive.py	2012-05-14 05:23:21 +0000
@@ -514,7 +514,7 @@
             SourcePackagePublishingHistory.archiveID == self.id,
             SourcePackagePublishingHistory.sourcepackagereleaseID ==
                 SourcePackageRelease.id,
-            SourcePackageRelease.sourcepackagenameID ==
+            SourcePackagePublishingHistory.sourcepackagenameID ==
                 SourcePackageName.id,
             ]
         orderBy = [
@@ -624,7 +624,7 @@
             SourcePackagePublishingHistory.archive = %s AND
             SourcePackagePublishingHistory.sourcepackagerelease =
                 SourcePackageRelease.id AND
-            SourcePackageRelease.sourcepackagename =
+            SourcePackagePublishingHistory.sourcepackagename =
                 SourcePackageName.id
         """ % sqlvalues(self)]
 
@@ -2493,9 +2493,8 @@
             SourcePackagePublishingHistory.archive == Archive.id,
             (SourcePackagePublishingHistory.status ==
                 PackagePublishingStatus.PUBLISHED),
-            (SourcePackagePublishingHistory.sourcepackagerelease ==
-                SourcePackageRelease.id),
-            SourcePackageRelease.sourcepackagename == source_package_name,
+            SourcePackagePublishingHistory.sourcepackagename ==
+                source_package_name,
             SourcePackagePublishingHistory.distroseries == DistroSeries.id,
             DistroSeries.distribution == distribution,
             )

=== modified file 'lib/lp/soyuz/model/distributionsourcepackagecache.py'
--- lib/lp/soyuz/model/distributionsourcepackagecache.py	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/model/distributionsourcepackagecache.py	2012-05-14 05:23:21 +0000
@@ -95,9 +95,7 @@
             DistroSeries.distribution = %s AND
             Archive.id = %s AND
             SourcePackagePublishingHistory.archive = Archive.id AND
-            SourcePackagePublishingHistory.sourcepackagerelease =
-                SourcePackageRelease.id AND
-            SourcePackageRelease.sourcepackagename =
+            SourcePackagePublishingHistory.sourcepackagename =
                 SourcePackageName.id AND
             SourcePackagePublishingHistory.dateremoved is NULL AND
             Archive.enabled = TRUE
@@ -106,8 +104,7 @@
             clauseTables=[
                 'Archive',
                 'DistroSeries',
-                'SourcePackagePublishingHistory',
-                'SourcePackageRelease']))
+                'SourcePackagePublishingHistory']))
 
         # Remove the cache entries for packages we no longer publish.
         for cache in cls._find(distro, archive):
@@ -128,9 +125,9 @@
 
         # Get the set of published sourcepackage releases.
         sprs = list(SourcePackageRelease.select("""
-            SourcePackageRelease.sourcepackagename = %s AND
             SourcePackageRelease.id =
                 SourcePackagePublishingHistory.sourcepackagerelease AND
+            SourcePackagePublishingHistory.sourcepackagename = %s AND
             SourcePackagePublishingHistory.distroseries =
                 DistroSeries.id AND
             DistroSeries.distribution = %s AND
@@ -219,16 +216,13 @@
                 DistroSeries.id AND
             DistroSeries.distribution = %s AND
             SourcePackagePublishingHistory.archive = %s AND
-            SourcePackagePublishingHistory.sourcepackagerelease =
-                SourcePackageRelease.id AND
-            SourcePackageRelease.sourcepackagename =
+            SourcePackagePublishingHistory.sourcepackagename =
                 SourcePackageName.id AND
             SourcePackagePublishingHistory.dateremoved is NULL
             """ % sqlvalues(distro, archive),
             distinct=True,
             orderBy="name",
-            clauseTables=['SourcePackagePublishingHistory', 'DistroSeries',
-                'SourcePackageRelease']))
+            clauseTables=['SourcePackagePublishingHistory', 'DistroSeries']))
 
         number_of_updates = 0
         chunk_size = 0

=== modified file 'lib/lp/soyuz/model/distroseriesdifferencejob.py'
--- lib/lp/soyuz/model/distroseriesdifferencejob.py	2012-04-13 20:58:40 +0000
+++ lib/lp/soyuz/model/distroseriesdifferencejob.py	2012-05-14 05:23:21 +0000
@@ -41,7 +41,6 @@
     DistributionJobDerived,
     )
 from lp.soyuz.model.publishing import SourcePackagePublishingHistory
-from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 
 
 FEATURE_FLAG_ENABLE_MODULE = u"soyuz.derived_series_jobs.enabled"
@@ -85,21 +84,17 @@
         `derived_series`.
     :return: A list of newly-created `DistributionJob` ids.
     """
-    store = IStore(SourcePackageRelease)
-    source_package_releases = store.find(
-        SourcePackageRelease,
-        SourcePackagePublishingHistory.sourcepackagerelease ==
-            SourcePackageRelease.id,
+    store = IStore(SourcePackagePublishingHistory)
+    spn_ids = store.find(
+        SourcePackagePublishingHistory.sourcepackagenameID,
         SourcePackagePublishingHistory.distroseries == derived_series.id,
         SourcePackagePublishingHistory.status.is_in(active_publishing_status))
-    nb_jobs = source_package_releases.count()
+    spn_ids = list(spn_ids)
 
-    if nb_jobs == 0:
+    if len(spn_ids) == 0:
         return []
 
-    sourcepackagenames = source_package_releases.values(
-        SourcePackageRelease.sourcepackagenameID)
-    job_ids = Job.createMultiple(store, nb_jobs)
+    job_ids = Job.createMultiple(store, len(spn_ids))
     return bulk.create(
             (DistributionJob.distribution, DistributionJob.distroseries,
              DistributionJob.job_type, DistributionJob.job_id,
@@ -107,7 +102,7 @@
             [(derived_series.distribution, derived_series,
               DistributionJobType.DISTROSERIESDIFFERENCE, job_id,
               make_metadata(spn_id, parent_series.id))
-             for job_id, spn_id in zip(job_ids, sourcepackagenames)],
+             for job_id, spn_id in zip(job_ids, spn_ids)],
             get_primary_keys=True)
 
 

=== modified file 'lib/lp/soyuz/tests/test_hasbuildrecords.py'
--- lib/lp/soyuz/tests/test_hasbuildrecords.py	2012-04-16 23:02:44 +0000
+++ lib/lp/soyuz/tests/test_hasbuildrecords.py	2012-05-14 05:23:21 +0000
@@ -22,6 +22,7 @@
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.model.sourcepackage import SourcePackage
+from lp.services.database.lpstorm import IStore
 from lp.soyuz.enums import ArchivePurpose
 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuild
 from lp.soyuz.interfaces.buildrecords import (
@@ -29,6 +30,7 @@
     IncompatibleArguments,
     )
 from lp.soyuz.model.processor import ProcessorFamilySet
+from lp.soyuz.model.publishing import SourcePackagePublishingHistory
 from lp.soyuz.tests.test_binarypackagebuild import BaseTestCaseWithThreeBuilds
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
 from lp.testing import (
@@ -281,6 +283,9 @@
         for build in self.builds[1:3]:
             spr = build.source_package_release
             removeSecurityProxy(spr).sourcepackagename = gedit_name
+            IStore(SourcePackagePublishingHistory).find(
+                SourcePackagePublishingHistory, sourcepackagerelease=spr
+                ).set(sourcepackagenameID=gedit_name.id)
 
         # Set them as sucessfully built
         for build in self.builds:

=== modified file 'lib/lp/translations/model/vpoexport.py'
--- lib/lp/translations/model/vpoexport.py	2011-12-24 17:49:30 +0000
+++ lib/lp/translations/model/vpoexport.py	2012-05-14 05:23:21 +0000
@@ -26,7 +26,6 @@
     )
 from lp.soyuz.model.component import Component
 from lp.soyuz.model.publishing import SourcePackagePublishingHistory
-from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 from lp.translations.interfaces.vpoexport import (
     IVPOExport,
     IVPOExportSet,
@@ -66,16 +65,13 @@
         if component is not None:
             tables.extend([
                 SourcePackagePublishingHistory,
-                SourcePackageRelease,
                 Component,
                 ])
             conditions.extend([
                 SourcePackagePublishingHistory.distroseries == series,
-                SourcePackagePublishingHistory.sourcepackagerelease ==
-                     SourcePackageRelease.id,
                 SourcePackagePublishingHistory.component == Component.id,
                 POTemplate.sourcepackagename ==
-                    SourcePackageRelease.sourcepackagenameID,
+                    SourcePackagePublishingHistory.sourcepackagenameID,
                 Component.name == component,
                 SourcePackagePublishingHistory.dateremoved == None,
                 SourcePackagePublishingHistory.archive == series.main_archive,