launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05364
[Merge] lp:~jtv/launchpad/bug-884649-branch-1 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-884649-branch-1 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #884649 in Launchpad itself: "Ubuntu publisher is taking more than an hour to complete"
https://bugs.launchpad.net/launchpad/+bug/884649
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-884649-branch-1/+merge/80988
= Summary =
The Dominator is having performance problems, now that we've introduced binary double-domination. (It's probably not as exciting as it sounds, but it'll fix a lot of uninstallable packages.)
Getting dominator performance under control needs some careful but radical internal reorganizations.
== Proposed fix ==
Prepare for some of the coming changes (yes, there's a bug-884649-branch-2 in the works) and, while we're at it, cut some dead wood out of the main query involved in the added functionality.
That main query is executed for almost any binary package publication at some point in its lifetime. The dead wood is two tables being joined through SourcePackageRelease, when all that's needed is an equijoin on the foreign keys to SourcePackageRelease. The table itself isn't needed in the join; this was easy to miss given the complexity of the search.
== Pre-implementation notes ==
Discussed extensively with Julian. see bug 884649 for notes.
== Implementation details ==
== Tests ==
I ran them all, actually. But anything with “dominator” or “domination” or “dominate” in the name is particularly likely to be relevant:
{{{
./bin/test -vvc -t dominat
}}}
== Demo and Q/A ==
The dominator should still work, and probably be just slightly faster. It's hard to benchmark accurately though, since every run will face different sets of active publications. We could just run it twice on a second server and time the second run; the second run won't be as representative because it has no changes to make, but at least it will run in predictable time.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/archivepublisher/domination.py
lib/lp/soyuz/model/publishing.py
--
https://code.launchpad.net/~jtv/launchpad/bug-884649-branch-1/+merge/80988
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-884649-branch-1 into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/domination.py'
--- lib/lp/archivepublisher/domination.py 2011-10-18 11:56:09 +0000
+++ lib/lp/archivepublisher/domination.py 2011-11-02 08:33:27 +0000
@@ -258,7 +258,8 @@
# 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
- # than the one being superseded.
+ # than the one being superseded. In this loop, that means the
+ # dominant is always the last live publication we saw.
publications = sorted(
publications, cmp=generalization.compare, reverse=True)
@@ -272,25 +273,29 @@
for pub in publications:
version = generalization.getPackageVersion(pub)
# There should never be two published releases with the same
- # version. So this comparison is really a string
- # comparison, not a version comparison: if the versions are
- # equal by either measure, they're from the same release.
- if dominant_version is not None and version == dominant_version:
+ # version. So it doesn't matter whether this comparison is
+ # really a string comparison or a version comparison: if the
+ # versions are equal by either measure, they're from the same
+ # release.
+ if version == dominant_version:
# This publication is for a live version, but has been
# superseded by a newer publication of the same version.
# Supersede it.
pub.supersede(current_dominant, logger=self.logger)
self.logger.debug2(
"Superseding older publication for version %s.", version)
- elif (version in live_versions or
- (not generalization.is_source and
- not self._checkArchIndep(pub))):
+ elif version in live_versions:
# This publication stays active; if any publications
# that follow right after this are to be superseded,
# this is the release that they are superseded by.
current_dominant = pub
dominant_version = version
self.logger.debug2("Keeping version %s.", version)
+ elif not (generalization.is_source or self._checkArchIndep(pub)):
+ # As a special case, we keep this version live as well.
+ current_dominant = pub
+ dominant_version = version
+ self.logger.debug2("Keeping version %s.", version)
elif current_dominant is None:
# This publication is no longer live, but there is no
# newer version to supersede it either. Therefore it
@@ -442,72 +447,76 @@
# always equals to "scheduleddeletiondate - quarantine".
pub_record.datemadepending = UTC_NOW
+ def findBinariesForDomination(self, distroarchseries, pocket):
+ """Find binary publications that need dominating."""
+ # Avoid circular imports.
+ from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
+
+ bpph_location_clauses = [
+ BinaryPackagePublishingHistory.status ==
+ PackagePublishingStatus.PUBLISHED,
+ BinaryPackagePublishingHistory.distroarchseries ==
+ distroarchseries,
+ BinaryPackagePublishingHistory.archive == self.archive,
+ BinaryPackagePublishingHistory.pocket == pocket,
+ ]
+ candidate_binary_names = Select(
+ BinaryPackageName.id,
+ And(
+ BinaryPackageRelease.binarypackagenameID ==
+ BinaryPackageName.id,
+ BinaryPackagePublishingHistory.binarypackagereleaseID ==
+ BinaryPackageRelease.id,
+ bpph_location_clauses,
+ ),
+ group_by=BinaryPackageName.id,
+ having=Count(BinaryPackagePublishingHistory.id) > 1)
+ main_clauses = [
+ BinaryPackageRelease.id ==
+ BinaryPackagePublishingHistory.binarypackagereleaseID,
+ BinaryPackageRelease.binarypackagenameID.is_in(
+ candidate_binary_names),
+ BinaryPackageRelease.binpackageformat !=
+ BinaryPackageFormat.DDEB,
+ ]
+ main_clauses.extend(bpph_location_clauses)
+
+ store = IStore(BinaryPackagePublishingHistory)
+ return store.find(BinaryPackagePublishingHistory, *main_clauses)
+
def dominateBinaries(self, distroseries, pocket):
"""Perform domination on binary package publications.
Dominates binaries, restricted to `distroseries`, `pocket`, and
`self.archive`.
"""
- # Avoid circular imports.
- from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
-
generalization = GeneralizedPublication(is_source=False)
for distroarchseries in distroseries.architectures:
self.logger.info(
"Performing domination across %s/%s (%s)",
- distroseries.name, pocket.title,
+ distroarchseries.distroseries.name, pocket.title,
distroarchseries.architecturetag)
- bpph_location_clauses = [
- BinaryPackagePublishingHistory.status ==
- PackagePublishingStatus.PUBLISHED,
- BinaryPackagePublishingHistory.distroarchseries ==
- distroarchseries,
- BinaryPackagePublishingHistory.archive == self.archive,
- BinaryPackagePublishingHistory.pocket == pocket,
- ]
- candidate_binary_names = Select(
- BinaryPackageName.id,
- And(
- BinaryPackageRelease.binarypackagenameID ==
- BinaryPackageName.id,
- BinaryPackagePublishingHistory.binarypackagereleaseID ==
- BinaryPackageRelease.id,
- bpph_location_clauses,
- ),
- group_by=BinaryPackageName.id,
- having=Count(BinaryPackagePublishingHistory.id) > 1)
- main_clauses = [
- BinaryPackagePublishingHistory,
- BinaryPackageRelease.id ==
- BinaryPackagePublishingHistory.binarypackagereleaseID,
- BinaryPackageRelease.binarypackagenameID.is_in(
- candidate_binary_names),
- BinaryPackageRelease.binpackageformat !=
- BinaryPackageFormat.DDEB,
- ]
- main_clauses.extend(bpph_location_clauses)
-
- def do_domination(pass2_msg=""):
- msg = "binaries..." + pass2_msg
- self.logger.info("Finding %s" % msg)
- bins = IStore(
- BinaryPackagePublishingHistory).find(*main_clauses)
- self.logger.info("Dominating %s" % msg)
- sorted_packages = self._sortPackages(bins, generalization)
- self._dominatePublications(sorted_packages, generalization)
-
- do_domination()
-
- # We need to make a second pass to cover the cases where:
- # * arch-specific binaries were not all dominated before arch-all
- # ones that depend on them
- # * An arch-all turned into an arch-specific, or vice-versa
- # * A package is completely schizophrenic and changes all of
- # its binaries between arch-all and arch-any (apparently
- # occurs sometimes!)
- do_domination("(2nd pass)")
+ self.logger.info("Finding binaries...")
+ bins = self.findBinariesForDomination(distroarchseries, pocket)
+ sorted_packages = self._sortPackages(bins, generalization)
+ self.logger.info("Dominating binaries...")
+ self._dominatePublications(sorted_packages, generalization)
+
+ # We need to make a second pass to cover the cases where:
+ # * arch-specific binaries were not all dominated before arch-all
+ # ones that depend on them
+ # * An arch-all turned into an arch-specific, or vice-versa
+ # * A package is completely schizophrenic and changes all of
+ # its binaries between arch-all and arch-any (apparently
+ # occurs sometimes!)
+ for distroarchseries in distroseries.architectures:
+ self.logger.info("Finding binaries...(2nd pass)")
+ bins = self.findBinariesForDomination(distroarchseries, pocket)
+ sorted_packages = self._sortPackages(bins, generalization)
+ self.logger.info("Dominating binaries...(2nd pass)")
+ self._dominatePublications(sorted_packages, generalization)
def _composeActiveSourcePubsCondition(self, distroseries, pocket):
"""Compose ORM condition for restricting relevant source pubs."""
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2011-10-23 02:58:56 +0000
+++ lib/lp/soyuz/model/publishing.py 2011-11-02 08:33:27 +0000
@@ -33,7 +33,6 @@
Desc,
LeftJoin,
Or,
- Select,
Sum,
)
from storm.store import Store
@@ -1154,19 +1153,10 @@
# Avoid circular wotsits.
from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
from lp.soyuz.model.distroarchseries import DistroArchSeries
- from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
- source_select = Select(
- SourcePackageRelease.id,
- And(
- BinaryPackageBuild.source_package_release_id ==
- SourcePackageRelease.id,
- BinaryPackageRelease.build == BinaryPackageBuild.id,
- self.binarypackagereleaseID == BinaryPackageRelease.id,
- ))
+
pubs = [
BinaryPackageBuild.source_package_release_id ==
- SourcePackageRelease.id,
- SourcePackageRelease.id.is_in(source_select),
+ self.binarypackagerelease.build.source_package_release_id,
BinaryPackageRelease.build == BinaryPackageBuild.id,
BinaryPackagePublishingHistory.binarypackagereleaseID ==
BinaryPackageRelease.id,
Follow ups
