Merge lp:~jtv/launchpad/bug-181368-parallelize into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Jeroen T. Vermeulen |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12474 |
Proposed branch: | lp:~jtv/launchpad/bug-181368-parallelize |
Merge into: | lp:launchpad |
Diff against target: |
246 lines (+143/-34) 2 files modified
lib/lp/archivepublisher/model/ftparchive.py (+39/-15) lib/lp/archivepublisher/tests/test_ftparchive.py (+104/-19) |
To merge this branch: | bzr merge lp:~jtv/launchpad/bug-181368-parallelize |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Leonard Richardson (community) | Approve | ||
Graham Binns (community) | code | Approve | |
Review via email: mp+48903@code.launchpad.net |
Commit message
[r=gmb,leonardr][bug=181368] Parallel apt-ftparchive.
Description of the change
= Summary =
The archive publisher takes far longer to run than anyone would like.
It spends most of its time running apt-ftparchive, which writes index files for each CPU architecture in turn.
== Proposed fix ==
This branch parallelizes the apt-ftparchive process into concurrent invocations, one for each architecture. It's made easy by the earlier introduction of the CommandSpawner, a helper class that lets you issue commands in parallel and then wait for them all to complete.
== Pre-implementation notes ==
The idea was wgrant's. Since this is a little scary, I also discussed the idea with bigjools. To validate the operational side, mthaddon observed the memory footprint of a full apt-ftparchive run. It took up less than a percent of available memory.
== Implementation details ==
The command-line option that lets us limit apt-ftparchive to a single architecture was introduced in Maverick. Previous versions allowed the same thing through the configuration file, but using it that way would have involved rewriting configuration files. Backpatching the new option was considered the lesser risk.
For the purposes of apt-ftparchive, "source" is just another architecture.
== Tests ==
To exercise the change:
{{{
./bin/test -vvc lp.archivepubli
}}}
I removed a few assertions on runApt's return value. The method used to return apt-ftparchive's exit code, but it would raise an exception first if that exit code was not zero. Therefore it always returned zero. I removed it. A new test verifies that runApt does indeed raise an exception if the command fails.
== Demo and Q/A ==
This is not something we can easily reproduce without Soyuz skills. I rely entirely on Julian there.
He has already tested the change on dogfood, though only with one architecture (plus the "source" one, of course). We'll have to try a larger run with multiple architectures as part of regular Q/A.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
So there!
From IRC:
[13:57] gmb: jtv: Most of the tests in TestFTPArchiveR unApt need comments, if you please. Otherwise I'm happy with the code, but I'm not convinced that I actually have enough domain knowledge to review this - my brain seems to keep skipping off it.
[13:58] gmb: jtv: I'll give it r=me, but you might want to get a review from someone who knows more about it than I do.