This repository was archived by the owner on Apr 19, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 43
improved image creation performance #198
Open
pohly
wants to merge
110
commits into
ostroproject:master
Choose a base branch
from
pohly:swupd-performance2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
87cc575 to
1826683
Compare
4d3b216 to
4257abc
Compare
Contributor
Author
|
test this please |
Contributor
Author
|
retest this please |
282c976 to
4257abc
Compare
Contributor
Author
|
retest this please |
Contributor
Author
|
retest this please Fetch error for openjdk-8-native in https://ostroproject.org/jenkins/job/build_beaglebone/2775/console ? |
Contributor
Author
|
retest this please |
08e420f to
a446ea2
Compare
Contributor
Author
|
retest this please |
6b2435e to
2400bb7
Compare
Maintaining a whitelist of kernel versions which need the setattr patches just doesn't work: swupd broke in Ostro OS when switching to kernel 4.4.14 (IOTOS-1712) and now again after switching to 4.4.20 (ostro-os/ostroproject#199). OTOH, not applying the patches by default enabled switching to 4.8 without having to do anything. Now that the patches are known to be included in Linux 4.7, we can and should apply the patches by default to all kernels older than that. Fixes: ostro-os/ostroproject#199 Signed-off-by: Patrick Ohly <[email protected]>
Image analysis is in the critical path for completing a build in Ostro OS because it runs as part of do_image_complete, and the analysis is relatively slow (minutes). Moving the check into a task allows image creation and the ensuing swupd update calculation to start sooner, while the image analysis runs in parallel. In addition, building just the image (do_image_complete) instead of the entire image recipe (do_build) no longer triggers the image check, which makes the default content of the build more useful: in Ostro, meta-swupd creates several virtual image recipes for which it runs do_image_complete. We could disable checking via image black- or whitelisting, but not running the task at all by default is easier. Signed-off-by: Patrick Ohly <[email protected]>
swupd-client 3.x no longer creates the /var/lib/swupd directory, so the previous workaround with chsmack on the directory stopped working when updating to 3.x. Related-to: ostroproject/ostro-os-xt@f4461ea Signed-off-by: Patrick Ohly <[email protected]>
1.0 has been released, but master already contains additional fixes and improvements, thus we update to the latest master. Also will allow us to patch it in a way that is immediately applicable upstream. Signed-off-by: Patrick Ohly <[email protected]>
This will allow speeding up image signing by invoking evmctl less often. Signed-off-by: Patrick Ohly <[email protected]>
do_image can spend quite some time (= minutes) while signing all files in the image, invoking evmctl one-by-one for each file. This gets faster (less process startup time, less overhead for connecting to pseudo) when processing as many files as possible per evmctl invocation. Depends on a new patch for evmctl. Signed-off-by: Patrick Ohly <[email protected]>
This makes it possible to import the master branch with combo-layer. Signed-off-by: Patrick Ohly <[email protected]>
This removes the storing of previous build information in sstate. It was conceptually questionable (sstate is a cache which does not need to be backed up, while the information about previous builds is crucial and must not get lost) and not working: - the -map.inc file wasn't actually included anywhere and thus the old build information wasn't getting restored - restoring all previous builds would have made building slower and slower as the number of previous builds grows - the old build information lacked the www/Manifest files that incremental updates need The replacement puts previous build information into the image deploy directory. That's tentative and also not fully working. The automatic selection of old versions to build deltas against also gets replaced with an explicit choice that has to be made by the user of meta-swupd. That's because in practice, incremental updates are more useful when prepared for the releases that actually run on the target device, like major milestones. Signed-off-by: Patrick Ohly <[email protected]>
The settings affecting the swupd client belong to the image, not the swupd client recipe. That way, different images can use different settings while sharing the same swupd client. Creating the bundles directory was broken in the swupd-client recipe and also not needed because swupd-image.bbclass does it, too. This will also allow implementing better update repo generation (incremental, supporting format changes, etc.) because now swupd-image.bbclass has access to the settings. The installed swupd client must match the format of the update repo for that OS_VERSION. To ensure that, swupd-image.bbclass now adds a dependency on a virtual "swupd-client-format<format number>" and suitable swupd client recipe(s) provide that. Distros then have two ways of choosing a swupd client version, should that ever be necessary: - first they need to override the per-image format default value - then set the preferred swupd client version, if there is more than one for that format TODO: installing the SSL pubkey into the image after a file change does not work. Signed-off-by: Patrick Ohly <[email protected]>
The updated meta-swupd expects these settings to be for the image(s). Making them in the swupd client is conceptually wrong because different images may need different settings and/or developers may want to change them without recompiling swupd-client. We can set these values at the distro level instead of per-image because of the use of SWUPD_IMAGE_PN, the value is valid for different image recipes. Signed-off-by: Patrick Ohly <[email protected]>
Creating updates based on the Manifest.full of the previous build allows reusing unchanged files, i.e. work on compressing these file and the storing the result again under "files" gets avoided. This works by referencing the previous version in the new Manifest files. The implication of that is that versions no longer can be published separately. The content produced by all previous builds must also be available to the client. This is independent of computing deltas. Nothing besides the previous "www" content needs to be available. It gets downloaded automatically when starting a build without a previous swupd deploy directory, so no extra work is needed to enable this mode besides publishing the previous build results. Fixes [YOCTO #9189] Signed-off-by: Patrick Ohly <[email protected]>
A local copy of pseudo 1.7.5 was picked in 355b0a31c because of https://bugzilla.yoctoproject.org/show_bug.cgi?id=9929. The version of pseudo in OE-core no longer has that problem, so we can remove the workaround. Signed-off-by: Patrick Ohly <[email protected]>
pseudo 1.7.5 was added in c88e65c98 because of https://bugzilla.yoctoproject.org/show_bug.cgi?id=9929. The version of pseudo in OE-core no longer has that problem, so we can remove the workaround. Signed-off-by: Patrick Ohly <[email protected]>
pseudo_1.8.1.bb gets the backported patch and pseudo_git.bb gets updated to include the commit, which currently is the most recent commit on the pseudo master branch. There's just one problem - it breaks other cases: foo: security.SMACK64: No such attribute Signed-off-by: Patrick Ohly <[email protected]>
Required for meta-swupd performance enhancements: in meta-swupd, the so called "mega" image contains a rootfs with all files that can potentially be installed on a device. Other virtual image recipes need a subset of those files or directories, and a partial extraction from a single tar archive is faster than letting all virtual image recipes share access to a directory under a single pseudo instance. It may be necessary to extract a directory with all of its attributes without the content of the directory, hence this patch. TODO: decide about upstreaming it - upstream is willing to consider a patch (see https://groups.google.com/forum/#!topic/libarchive-discuss/JO3hqSaAVfs). Signed-off-by: Patrick Ohly <[email protected]>
Instead of granting all virtual images access to the mega rootfs under
a shared pseudo instance, archive the mega rootfs in an archive and
extract from that the subset of entries that are needed.
Sharing pseudo instances is slow: using more than one avoids a
potential bottleneck (the pseudo daemon is often 100% busy on a CPU
during heavy use). Extracting files with full attributes also is
faster when merely sweeping through a tar archive, at least when most
of the content is needed.
This change therefore increases performance.
bsdtar with support for --no-recursive in combination with -x is
needed for that. Current bsdtar master does not support that yet, but
adding it was easy.
GNU tar already supports that, but had bugs in that mode ("Not found
in archive" errors for entries that were in the archive).
bsdtar is also nicer for other reasons and therefore was extended instead
of trying to fix GNU tar:
- no need to explicitly add xattrs
- guaranteed to auto-detect compression, even when reading from
stdin (GNU tar can only do that when working with files); not
that relevant here, though
- uses less system calls when creating files, which should
help a bit with performance under pseudo
Signed-off-by: Patrick Ohly <[email protected]>
The IMA and Smack xattrs of the downloaded Manifest files are set on the downloaded and unpacked Manifest files, while the server doesn't have them at all. They need to be ignored. Signed-off-by: Patrick Ohly <[email protected]>
It is better to start each OS_VERSION build with a clean pseudo database because then performance is expected to be better. Only relevant for repeated local builds; CI builds already start from scratch. Signed-off-by: Patrick Ohly <[email protected]>
swupd_create_fullfiles segfaulted when no new files were needed for the current build because nothing changed. Very unlikely, but can happen during testing. Signed-off-by: Patrick Ohly <[email protected]>
In the case that a diff against a previous build is computed and both old and new files have the same xattr, the server failed because an internal sanity check was implemented incorrectly. Not relevant at the moment for Ostro OS because changing file content implies changing the ima.security xattr, in which case current swupd-server skips diffing entirely (changing xattrs via patching not supported). It's more relevant for Ostro OS XT, which has Smack, but not IMA. Signed-off-by: Patrick Ohly <[email protected]>
Not removing the directories is okay: typically we don't build incrementally, and we can remove any remaining ones before invoking swupd. Not removing a tempory directory tree may also have performance benefits, but the even better solution will be to not write the tree in the first place by calling libarchive directly. Related-to: https://bugzilla.yoctoproject.org/show_bug.cgi?id=10623 Signed-off-by: Patrick Ohly <[email protected]>
Putting the database under the deploy directory automatically makes it specific to the OS_VERSION and removing the deploy directory also removes the corresponding pseudo database, thus ensuring a clean rebuild with a single command. Signed-off-by: Patrick Ohly <[email protected]>
The previous approaches all relied on somehow carrying additional data across from one build to the next (sstate or additional archives in the deploy directory). The new approach replaces that with downloading required content on a file-by-file basis from the normal update repo when (and not sooner) it is needed by swupd_create_pack. That works for meta-swupd because the format of the files (compressed archive created with bsdtar) is expected to be stable. If a change ever becomes necessary, some backward compatibilty mode would have to be added or deltas simply would be skipped again. Signed-off-by: Patrick Ohly <[email protected]>
When IMA is active, the kernel ended up updating the file hash each time swupd wrote a chunk, because files were getting opened and closed for each chunk. Now they get opened before downloading and closed when done. Fixes: clearlinux/swupd-client/ostroproject#41 Signed-off-by: Patrick Ohly <[email protected]>
The kernel hardening causes bsdtar to report errors about security.ima when unpacking files for swupd. Depending on the swupd client (see clearlinux/swupd-client#156) that completely breaks updating. Signed-off-by: Patrick Ohly <[email protected]>
A crude workaround for [YOCTO #19791]: the token handed down from the parser isn't quite what the handler expects. Probably a bug in the parser. Required for the parallel execution of several commands in meta-swupd do_swupd_update. Signed-off-by: Patrick Ohly <[email protected]>
Internally, swupd_make_pack mostly just spends its time on a single CPU while compressing a large .tar archive. We can shorten the overall execution time by running swupd_make_pack invocations in parallel. The current approach just runs all of them at once. This might overload small machines or larger ones when the number of bundles increases, so some more intelligence might be needed. Depends on a fix for background processes in the bitbake shell parser (YOCTO #10668). Signed-off-by: Patrick Ohly <[email protected]>
libmagic is provided by file-native and required by swupd-server-native, but because file-native is usually assumed to be provided, it won't get compiled unless we depend on the special file-replacement-native. Signed-off-by: Patrick Ohly <[email protected]>
They are neither used nor supported. Signed-off-by: Patrick Ohly <[email protected]>
Besides being an integer, it also must be in the signed int32 range supported by swupd. Signed-off-by: Patrick Ohly <[email protected]>
There are reasons for format changes, the upcoming tools update being one of them. When the format changes, swupd-image.bbclass must build two OS versions from the same rootfs: once with the old format, once with the new format. OS_VERSION is used as number for the new format, OS_VERSION - 1 for the old one. OS_VERSION must be high enough such that OS_VERSION - 1 is still available. Usually it is, but there's also a sanity check for that. When changing the format because of a change in the tools, then both old and new swupd-server are needed, so now recipe and installed files include the tool format version. Signed-off-by: Patrick Ohly <[email protected]>
The two need to be updated in lockstep because of tool format changes, leading to tools format 4. Because of that, we need to keep the previous swupd-server around so that we can generate intermediate versions for images with the previous client. This way, an Ostro OS 4400 (= 1.0) image can update to an intermediate version with the new swupd client and from there to more recent versions. swupd client 3.7.2 has two of our patches merged (0001-downloads-minimize-syscalls-to-improve-performance.patch, 0002-downloads-open-FILE-in-advance-and-use-default-write.patch) and changes the HTTPS certificate checking so that the download uses the system's trusted CAs instead of hard-coding a single one, so Make-pinned-pubkey-configurable.patch is no longer needed. Furthermore, the out-of-tree compilation was fixed upstream, so we can drop the autotools-brokensep workaround. swupd server 3.3.0 has merged 0001-delta.c-fix-xattr-test-after-patching.patch and swupd_create_fullfiles-avoid-segfault-when-nothing-c.patch. Signed-off-by: Patrick Ohly <[email protected]>
Creating the mega image archive is on the critical path (depends on all target components having been compiled and blocks creating images). Compressing, even with pbzip, is slower than directly writing the uncompressed archive (tested with a striped RAID array of two traditional hard drives and a fast multicore CPU) and decompression again takes additional time, so avoid the slowdown by not compressing. The downside is higher disk space usage. Signed-off-by: Patrick Ohly <[email protected]>
A local patch is needed for updating Ostro OS 1.0 until upstream officially supports this. Signed-off-by: Patrick Ohly <[email protected]>
The previous approach reused content from the latest build. The goal was to make the build as fast as possible by maximizing reuse. However, current Ostro OS release procedures do not guarantee that the "builds" directory is kept around and therefore reusing content from it is not currently possible. These procedures could be changed, but until then we have to use the "milestone" directory. Signed-off-by: Patrick Ohly <[email protected]>
6c139ae to
4b0908a
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces PR #190.