launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02833
[Merge] lp:~wgrant/launchpad/bug-728246-copy-expiry-queries into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-728246-copy-expiry-queries into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#728246 CopyChecker.checkCopy's expiration check issues multiple queries for each binary
https://bugs.launchpad.net/bugs/728246
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-728246-copy-expiry-queries/+merge/52020
This branch starts to address the package copier's scaling problems, making the expired binary check take a constant number of queries (bug #728246).
BinaryPackageRelease.files is now a cached list. getBuiltBinaries has been Stormified, had its prejoin replaced with a separate BPR preload query, and now takes an option to preload the BPFs and LFAs. I added a unit test to ensure that the query count is constant and that the preloading works.
This eliminates two queries per copied binary publication.
--
https://code.launchpad.net/~wgrant/launchpad/bug-728246-copy-expiry-queries/+merge/52020
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-728246-copy-expiry-queries into lp:launchpad.
=== modified file 'lib/lp/soyuz/model/binarypackagerelease.py'
--- lib/lp/soyuz/model/binarypackagerelease.py 2010-12-24 02:22:11 +0000
+++ lib/lp/soyuz/model/binarypackagerelease.py 2011-03-03 09:01:49 +0000
@@ -22,6 +22,7 @@
Date,
Int,
Reference,
+ Store,
Storm,
)
from zope.interface import implements
@@ -35,6 +36,10 @@
SQLBase,
sqlvalues,
)
+from lp.services.propertycache import (
+ cachedproperty,
+ get_property_cache,
+ )
from lp.soyuz.enums import (
BinaryPackageFileType,
BinaryPackageFormat,
@@ -84,9 +89,6 @@
debug_package = ForeignKey(dbName='debug_package',
foreignKey='BinaryPackageRelease')
- files = SQLMultipleJoin('BinaryPackageFile',
- joinColumn='binarypackagerelease', orderBy="libraryfile")
-
_user_defined_fields = StringCol(dbName='user_defined_fields')
def __init__(self, *args, **kwargs):
@@ -136,6 +138,11 @@
self.binarypackagename)
return distroarchseries_binary_package.currentrelease is None
+ @cachedproperty
+ def files(self):
+ return list(
+ Store.of(self).find(BinaryPackageFile, binarypackagerelease=self))
+
def addFile(self, file):
"""See `IBinaryPackageRelease`."""
determined_filetype = None
@@ -151,6 +158,7 @@
raise AssertionError(
'Unsupported file type: %s' % file.filename)
+ del get_property_cache(self).files
return BinaryPackageFile(binarypackagerelease=self,
filetype=determined_filetype,
libraryfile=file)
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2011-02-22 22:14:32 +0000
+++ lib/lp/soyuz/model/publishing.py 2011-03-03 09:01:49 +0000
@@ -464,40 +464,51 @@
for source, binary_pub, binary, binary_name, arch
in result_set]
- def getBuiltBinaries(self):
+ def getBuiltBinaries(self, want_files=False):
"""See `ISourcePackagePublishingHistory`."""
- clauses = """
- BinaryPackagePublishingHistory.binarypackagerelease=
- BinaryPackageRelease.id AND
- BinaryPackagePublishingHistory.distroarchseries=
- DistroArchSeries.id AND
- BinaryPackageRelease.build=BinaryPackageBuild.id AND
- BinaryPackageBuild.source_package_release=%s AND
- DistroArchSeries.distroseries=%s AND
- BinaryPackagePublishingHistory.archive=%s AND
- BinaryPackagePublishingHistory.pocket=%s
- """ % sqlvalues(self.sourcepackagerelease, self.distroseries,
- self.archive, self.pocket)
-
- clauseTables = [
- 'BinaryPackageBuild', 'BinaryPackageRelease', 'DistroArchSeries']
- orderBy = ['-BinaryPackagePublishingHistory.id']
- preJoins = ['binarypackagerelease']
-
- results = BinaryPackagePublishingHistory.select(
- clauses, orderBy=orderBy, clauseTables=clauseTables,
- prejoins=preJoins)
- binary_publications = list(results)
-
- unique_binary_ids = set(
- [pub.binarypackagerelease.id for pub in binary_publications])
+ from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
+ from lp.soyuz.model.distroarchseries import DistroArchSeries
+ binary_publications = list(Store.of(self).find(
+ BinaryPackagePublishingHistory,
+ BinaryPackagePublishingHistory.binarypackagereleaseID ==
+ BinaryPackageRelease.id,
+ BinaryPackagePublishingHistory.distroarchseriesID ==
+ DistroArchSeries.id,
+ BinaryPackagePublishingHistory.archiveID == self.archiveID,
+ BinaryPackagePublishingHistory.pocket == self.pocket,
+ BinaryPackageBuild.id == BinaryPackageRelease.buildID,
+ BinaryPackageBuild.source_package_release_id ==
+ self.sourcepackagereleaseID,
+ DistroArchSeries.distroseriesID == self.distroseriesID))
+
+ # Preload attached BinaryPackageReleases.
+ bpr_ids = set(
+ pub.binarypackagereleaseID for pub in binary_publications)
+ list(Store.of(self).find(
+ BinaryPackageRelease, BinaryPackageRelease.id.is_in(bpr_ids)))
+
+ if want_files:
+ # Preload BinaryPackageFiles.
+ bpfs = list(Store.of(self).find(
+ BinaryPackageFile,
+ BinaryPackageFile.binarypackagereleaseID.is_in(bpr_ids)))
+ bpfs_by_bpr = defaultdict(list)
+ for bpf in bpfs:
+ bpfs_by_bpr[bpf.binarypackagerelease].append(bpf)
+ for bpr in bpfs_by_bpr:
+ get_property_cache(bpr).files = bpfs_by_bpr[bpr]
+
+ # Preload LibraryFileAliases.
+ lfa_ids = set(bpf.libraryfileID for bpf in bpfs)
+ list(Store.of(self).find(
+ LibraryFileAlias, LibraryFileAlias.id.is_in(lfa_ids)))
unique_binary_publications = []
for pub in binary_publications:
- if pub.binarypackagerelease.id in unique_binary_ids:
+ if pub.binarypackagerelease.id in bpr_ids:
unique_binary_publications.append(pub)
- unique_binary_ids.remove(pub.binarypackagerelease.id)
- if len(unique_binary_ids) == 0:
+ bpr_ids.remove(pub.binarypackagerelease.id)
+ if len(bpr_ids) == 0:
break
return unique_binary_publications
=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py 2011-01-14 14:03:28 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py 2011-03-03 09:01:49 +0000
@@ -410,7 +410,7 @@
raise CannotCopy('source contains expired files')
if self.include_binaries:
- built_binaries = source.getBuiltBinaries()
+ built_binaries = source.getBuiltBinaries(want_files=True)
if len(built_binaries) == 0:
raise CannotCopy("source has no binaries to be copied")
# Deny copies of binary publications containing files with
=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py 2011-01-15 00:05:55 +0000
+++ lib/lp/soyuz/tests/test_publishing.py 2011-03-03 09:01:49 +0000
@@ -11,6 +11,8 @@
import tempfile
import pytz
+from storm.store import Store
+from testtools.matchers import Equals
import transaction
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -58,9 +60,10 @@
SourcePackagePublishingHistory,
)
from lp.testing import (
- login_as,
+ StormStatementRecorder,
TestCaseWithFactory,
)
+from lp.testing.matchers import HasQueryCount
from lp.testing.factory import LaunchpadObjectFactory
@@ -1362,3 +1365,37 @@
spph = self.factory.makeSourcePackagePublishingHistory(
ancestor=ancestor)
self.assertEquals(spph.ancestor.displayname, ancestor.displayname)
+
+
+class TestGetBuiltBinaries(TestNativePublishingBase):
+ """Test SourcePackagePublishingHistory.getBuiltBinaries() works."""
+
+ def test_flat_query_count(self):
+ spph = self.getPubSource(architecturehintlist='any')
+ store = Store.of(spph)
+ store.flush()
+ store.invalidate()
+
+ # An initial invocation issues one query for the each of the
+ # SPPH, BPPHs and BPRs.
+ with StormStatementRecorder() as recorder:
+ bins = spph.getBuiltBinaries()
+ self.assertEquals(0, len(bins))
+ self.assertThat(recorder, HasQueryCount(Equals(3)))
+
+ self.getPubBinaries(pub_source=spph)
+ store.flush()
+ store.invalidate()
+
+ # A subsequent invocation with files preloaded queries the SPPH,
+ # BPPHs, BPRs, BPFs and LFAs. Checking the filenames of each
+ # BPF has no query penalty.
+ with StormStatementRecorder() as recorder:
+ bins = spph.getBuiltBinaries(want_files=True)
+ self.assertEquals(2, len(bins))
+ for bpph in bins:
+ files = bpph.binarypackagerelease.files
+ self.assertEqual(1, len(files))
+ for bpf in files:
+ bpf.libraryfile.filename
+ self.assertThat(recorder, HasQueryCount(Equals(5)))
