launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05063
[Merge] lp:~jtv/launchpad/post-857155 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/post-857155 into lp:launchpad with lp:~jtv/launchpad/bug-857155 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #857155 in Launchpad itself: "Gina domination is slow"
https://bugs.launchpad.net/launchpad/+bug/857155
For more details, see:
https://code.launchpad.net/~jtv/launchpad/post-857155/+merge/76837
= Summary =
After the main optimization for Gina domination, here's another smaller one that may also help.
== Proposed fix ==
Two changes:
* Batch-fetch SPRs when dominating SPPHs (similar for BPR's/BPPRs).
* Rename dominateRemovedSourceVersions to dominateSourceVersions.
The name change is because a publication could be dominated not just because its version has been superseded, but also because the publication for a version has been replaced with a newer one for the same version.
== Pre-implementation notes ==
Came up as an extra in discussion and review for bug 857155.
== Implementation details ==
I had to extend the interface for BPPH to be a bit more like SPPH (which is the general thrust anyway) to accommodate load_related. I hope it's mostly self-explanatory; not much point in abstract explanations for a pair of very practical helpers. The two traits classes hide differences between SPPH and BPPH.
== Tests ==
{{{
./bin/test -vvc lp.soyuz.scripts.tests.test_gina
./bin/test -vvc lp.archivepublisher.tests.test_dominator
}}}
== Demo and Q/A ==
Will be combined with Q/A for the main 857155 branch.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/archivepublisher/domination.py
lib/lp/soyuz/scripts/gina/dominate.py
lib/lp/soyuz/model/publishing.py
lib/lp/soyuz/interfaces/publishing.py
lib/lp/archivepublisher/tests/test_dominator.py
--
https://code.launchpad.net/~jtv/launchpad/post-857155/+merge/76837
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/post-857155 into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/domination.py'
--- lib/lp/archivepublisher/domination.py 2011-09-24 06:02:25 +0000
+++ lib/lp/archivepublisher/domination.py 2011-09-24 06:02:25 +0000
@@ -69,6 +69,7 @@
)
from canonical.launchpad.interfaces.lpstorm import IStore
from lp.registry.model.sourcepackagename import SourcePackageName
+from lp.services.database.bulk import load_related
from lp.soyuz.enums import (
BinaryPackageFormat,
PackagePublishingStatus,
@@ -109,6 +110,9 @@
Used by `GeneralizedPublication` to hide the differences from
`BinaryPackagePublishingHistory`.
"""
+ release_class = SourcePackageRelease
+ release_reference_name = 'sourcepackagereleaseID'
+
@staticmethod
def getPackageName(spph):
"""Return the name of this publication's source package."""
@@ -126,6 +130,9 @@
Used by `GeneralizedPublication` to hide the differences from
`SourcePackagePublishingHistory`.
"""
+ release_class = BinaryPackageRelease
+ release_reference_name = 'binarypackagereleaseID'
+
@staticmethod
def getPackageName(bpph):
"""Return the name of this publication's binary package."""
@@ -156,9 +163,15 @@
return self.traits.getPackageName(pub)
def getPackageVersion(self, pub):
- """Obtain the version string for a publicaiton record."""
+ """Obtain the version string for a publication record."""
return self.traits.getPackageRelease(pub).version
+ def load_releases(self, pubs):
+ """Load the releases associated with a series of publications."""
+ return load_related(
+ self.traits.release_class, pubs,
+ [self.traits.release_reference_name])
+
def compare(self, pub1, pub2):
"""Compare publications by version.
@@ -217,6 +230,9 @@
:param generalization: A `GeneralizedPublication` helper representing
the kind of publications these are--source or binary.
"""
+ publications = list(publications)
+ generalization.load_releases(publications)
+
# Go through publications from latest version to oldest. This
# makes it easy to figure out which release superseded which:
# the dominant is always the oldest live release that is newer
@@ -558,8 +574,8 @@
Desc(SourcePackageRelease.version),
Desc(SourcePackagePublishingHistory.datecreated))
- def dominateRemovedSourceVersions(self, distroseries, pocket,
- package_name, live_versions):
+ def dominateSourceVersions(self, distroseries, pocket, package_name,
+ live_versions):
"""Dominate source publications based on a set of "live" versions.
Active publications for the "live" versions will remain active. All
=== modified file 'lib/lp/archivepublisher/tests/test_dominator.py'
--- lib/lp/archivepublisher/tests/test_dominator.py 2011-09-24 06:02:25 +0000
+++ lib/lp/archivepublisher/tests/test_dominator.py 2011-09-24 06:02:25 +0000
@@ -280,6 +280,19 @@
bpph.binarypackagerelease.version,
GeneralizedPublication(is_source=False).getPackageVersion(bpph))
+ def test_load_releases_loads_sourcepackagerelease(self):
+ spph = self.factory.makeSourcePackagePublishingHistory()
+ self.assertContentEqual(
+ [spph.sourcepackagerelease],
+ GeneralizedPublication(is_source=True).load_releases([spph]))
+
+ def test_load_releases_loads_binarypackagerelease(self):
+ bpph = self.factory.makeBinaryPackagePublishingHistory(
+ binarypackagerelease=self.factory.makeBinaryPackageRelease())
+ self.assertContentEqual(
+ [bpph.binarypackagerelease],
+ GeneralizedPublication(is_source=False).load_releases([bpph]))
+
def test_compare_sorts_versions(self):
versions = [
'1.1v2',
@@ -571,13 +584,13 @@
self.assertEqual(
expected_supersededby, [pub.supersededby for pub in pubs])
- def test_dominateRemovedSourceVersions_dominates_publications(self):
- # dominateRemovedSourceVersions finds the publications for a
- # package and calls dominatePackage on them.
+ def test_dominateSourceVersions_dominates_publications(self):
+ # dominateSourceVersions finds the publications for a package
+ # and calls dominatePackage on them.
pubs = make_spphs_for_versions(self.factory, ['0.1', '0.2', '0.3'])
package_name = pubs[0].sourcepackagerelease.sourcepackagename.name
- self.makeDominator(pubs).dominateRemovedSourceVersions(
+ self.makeDominator(pubs).dominateSourceVersions(
pubs[0].distroseries, pubs[0].pocket, package_name, ['0.2'])
self.assertEqual([
PackagePublishingStatus.SUPERSEDED,
@@ -589,21 +602,21 @@
[pubs[1].sourcepackagerelease, None, None],
[pub.supersededby for pub in pubs])
- def test_dominateRemovedSourceVersions_ignores_other_pockets(self):
- # dominateRemovedSourceVersions ignores publications in other
- # pockets than the one specified.
+ def test_dominateSourceVersions_ignores_other_pockets(self):
+ # dominateSourceVersions ignores publications in other pockets
+ # than the one specified.
pubs = make_spphs_for_versions(self.factory, ['2.3', '2.4'])
package_name = pubs[0].sourcepackagerelease.sourcepackagename.name
removeSecurityProxy(pubs[0]).pocket = PackagePublishingPocket.UPDATES
removeSecurityProxy(pubs[1]).pocket = PackagePublishingPocket.PROPOSED
- self.makeDominator(pubs).dominateRemovedSourceVersions(
+ self.makeDominator(pubs).dominateSourceVersions(
pubs[0].distroseries, pubs[0].pocket, package_name, ['2.3'])
self.assertEqual(PackagePublishingStatus.PUBLISHED, pubs[1].status)
- def test_dominateRemovedSourceVersions_ignores_other_packages(self):
+ def test_dominateSourceVersions_ignores_other_packages(self):
pubs = make_spphs_for_versions(self.factory, ['1.0', '1.1'])
other_package_name = self.factory.makeSourcePackageName().name
- self.makeDominator(pubs).dominateRemovedSourceVersions(
+ self.makeDominator(pubs).dominateSourceVersions(
pubs[0].distroseries, pubs[0].pocket, other_package_name, ['1.1'])
self.assertEqual(PackagePublishingStatus.PUBLISHED, pubs[0].status)
=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py 2011-09-23 07:44:44 +0000
+++ lib/lp/soyuz/interfaces/publishing.py 2011-09-24 06:02:25 +0000
@@ -381,7 +381,8 @@
))
archive = exported(
Reference(
- Interface, # Really IArchive, see below.
+ # Really IArchive (fixed in _schema_circular_imports.py).
+ Interface,
title=_('Archive ID'), required=True, readonly=True,
))
supersededby = Int(
@@ -478,7 +479,9 @@
"if one exists, or None.")
ancestor = Reference(
- Interface, # Really ISourcePackagePublishingHistory
+ # Really ISourcePackagePublishingHistory (fixed in
+ # _schema_circular_imports.py).
+ Interface,
title=_('Ancestor'),
description=_('The previous release of this source package.'),
required=False, readonly=True)
@@ -519,7 +522,8 @@
`IBinaryPackagePublishingHistory`.
"""
- @operation_returns_collection_of(Interface) # Really IBuild, see below.
+ # Really IBuild (fixed in _schema_circular_imports.py)
+ @operation_returns_collection_of(Interface)
@export_read_operation()
def getBuilds():
"""Return a list of `IBuild` objects in this publishing context.
@@ -688,12 +692,16 @@
title=_('The DB id for the binarypackagename.'),
required=False, readonly=False)
binarypackagename = Attribute('The binary package name being published')
- binarypackagerelease = Int(
- title=_('The binary package being published'), required=False,
- readonly=False)
+ binarypackagereleaseID = Int(
+ title=_('The DB id for the binarypackagerelease.'),
+ required=False, readonly=False)
+ binarypackagerelease = Attribute(
+ "The binary package release being published")
distroarchseries = exported(
Reference(
- Interface, #Really IDistroArchSeries, circular import fixed below.
+ # Really IDistroArchSeries (fixed in
+ #_schema_circular_imports.py).
+ Interface,
title=_("Distro Arch Series"),
description=_('The distroarchseries being published into'),
required=False, readonly=False,
@@ -779,7 +787,8 @@
exported_as="date_removed")
archive = exported(
Reference(
- Interface, # Really IArchive, see below.
+ # Really IArchive (fixed in _schema_circular_imports.py).
+ Interface,
title=_('Archive'),
description=_("The context archive for this publication."),
required=True, readonly=True,
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2011-09-23 07:47:04 +0000
+++ lib/lp/soyuz/model/publishing.py 2011-09-24 06:02:25 +0000
@@ -114,9 +114,8 @@
from lp.soyuz.scripts.changeoverride import ArchiveOverriderError
-# XXX cprov 2006-08-18: move it away, perhaps archivepublisher/pool.py
-
def makePoolPath(source_name, component_name):
+ # XXX cprov 2006-08-18: move it away, perhaps archivepublisher/pool.py
"""Return the pool path for a given source name and component name."""
from lp.archivepublisher.diskpool import poolify
return os.path.join(
@@ -421,12 +420,12 @@
"""A source package release publishing record."""
implements(ISourcePackagePublishingHistory)
- sourcepackagename = ForeignKey(foreignKey='SourcePackageName',
- dbName='sourcepackagename')
- sourcepackagerelease = ForeignKey(foreignKey='SourcePackageRelease',
- dbName='sourcepackagerelease')
- distroseries = ForeignKey(foreignKey='DistroSeries',
- dbName='distroseries')
+ sourcepackagename = ForeignKey(
+ foreignKey='SourcePackageName', dbName='sourcepackagename')
+ sourcepackagerelease = ForeignKey(
+ foreignKey='SourcePackageRelease', dbName='sourcepackagerelease')
+ distroseries = ForeignKey(
+ foreignKey='DistroSeries', dbName='distroseries')
component = ForeignKey(foreignKey='Component', dbName='component')
section = ForeignKey(foreignKey='Section', dbName='section')
status = EnumCol(schema=PackagePublishingStatus)
@@ -688,7 +687,7 @@
return self.distroseries.distribution.getSourcePackageRelease(
self.supersededby)
- # XXX: StevenK 2011-09-13 bug=848563: This can die when
+ # XXX: StevenK 2011-09-13 bug=848563: This can die when
# self.sourcepackagename is populated.
@property
def source_package_name(self):
@@ -912,12 +911,12 @@
implements(IBinaryPackagePublishingHistory)
- binarypackagename = ForeignKey(foreignKey='BinaryPackageName',
- dbName='binarypackagename')
- binarypackagerelease = ForeignKey(foreignKey='BinaryPackageRelease',
- dbName='binarypackagerelease')
- distroarchseries = ForeignKey(foreignKey='DistroArchSeries',
- dbName='distroarchseries')
+ binarypackagename = ForeignKey(
+ foreignKey='BinaryPackageName', dbName='binarypackagename')
+ binarypackagerelease = ForeignKey(
+ foreignKey='BinaryPackageRelease', dbName='binarypackagerelease')
+ distroarchseries = ForeignKey(
+ foreignKey='DistroArchSeries', dbName='distroarchseries')
component = ForeignKey(foreignKey='Component', dbName='component')
section = ForeignKey(foreignKey='Section', dbName='section')
priority = EnumCol(dbName='priority', schema=PackagePublishingPriority)
@@ -961,7 +960,7 @@
"""See `IBinaryPackagePublishingHistory`"""
return self.distroarchseries.distroseries
- # XXX: StevenK 2011-09-13 bug=848563: This can die when
+ # XXX: StevenK 2011-09-13 bug=848563: This can die when
# self.binarypackagename is populated.
@property
def binary_package_name(self):
=== modified file 'lib/lp/soyuz/scripts/gina/dominate.py'
--- lib/lp/soyuz/scripts/gina/dominate.py 2011-09-24 06:02:25 +0000
+++ lib/lp/soyuz/scripts/gina/dominate.py 2011-09-24 06:02:25 +0000
@@ -47,7 +47,7 @@
# it, dominating a single Debian series takes hours.
if pub_count != len(live_versions):
logger.debug("Dominating %s.", package_name)
- dominator.dominateRemovedSourceVersions(
+ dominator.dominateSourceVersions(
series, pocket, package_name, live_versions)
txn.commit()
else:
Follow ups
