Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[master] Large performance regression exporting layers with containerd worker #2365

Closed
aaronlehmann opened this issue Sep 15, 2021 · 11 comments · Fixed by #2388
Closed

[master] Large performance regression exporting layers with containerd worker #2365

aaronlehmann opened this issue Sep 15, 2021 · 11 comments · Fixed by #2388
Assignees
Milestone

Comments

@aaronlehmann
Copy link
Collaborator

aaronlehmann commented Sep 15, 2021

PR #2181 (Compute diff from the upper directory of overlayfs-based snapshotter) appears to cause a serious performance regression when exporting layers for an image push.

Before this merge commit, the "export layers" span of a relatively straightforward build/push took 8 seconds. On the merge commit, it is taking 69 seconds. containerd and buildkitd come close to maxing out CPU during this interval.

We are seeing a routine set of builds get dramatically slower, going from ~20 minutes end-to-end to about 85 minutes.

I can provide more information for debugging on request.

cc @ktock @tonistiigi @sipsma @coryb

@tonistiigi tonistiigi changed the title Large performance regression exporting layers [master] Large performance regression exporting layers Sep 15, 2021
@tonistiigi tonistiigi added this to the v0.10.0 milestone Sep 15, 2021
@aaronlehmann
Copy link
Collaborator Author

Just a random guess - could this be caused by a lack of buffering between the gzip compressor and the content store writer? I've run into similar issues with compress/gzip making small writes that turn out to be very inefficient.

I don't know if this is the issue here, though, and don't want to send you down the wrong path.

@aaronlehmann
Copy link
Collaborator Author

I did notice this comment on (*writer).Write in the local implementation of the content store:

// Write p to the transaction.
//
// Note that writes are unbuffered to the backing file. When writing, it is
// recommended to wrap in a bufio.Writer or, preferably, use io.CopyBuffer.
func (w *writer) Write(p []byte) (n int, err error) {

That suggests that maybe I'm on to something...

@ktock
Copy link
Collaborator

ktock commented Sep 16, 2021

@aaronlehmann Thank you for reporting this. I'm working on it.

Before this merge commit, the "export layers" span of a relatively straightforward build/push took 8 seconds.

Could you provide an example build reproduces this issue?

@ktock
Copy link
Collaborator

ktock commented Sep 16, 2021

PR #2181 (Compute diff from the upper directory of overlayfs-based snapshotter) appears to cause a serious performance regression when exporting layers for an image push.

This regression seems occur in containerd worker.
Tried the following simple Dockerfile (ghcr.io/stargz-containers/ubuntu:20.04-org is just a mirror of ubuntu:20.04)

FROM ghcr.io/stargz-containers/ubuntu:20.04-org
RUN apt-get update -y && apt-get install -y gcc

In containerd worker mode (w/ containerd v1.5.5 on ubuntu 20.04), "exporting layers" took 5.8s in 9462a23 (commit before the overlayfs differ) but took 18.2s in the master. (No regression occurred in OCI worker) I tried buffered copy (ktock@cc0ffec) for writing to the content store but it didn't solve this issue (still took 17.0s).

I did notice this comment on (*writer).Write in the local implementation of the content store:

Thank you for providing this. Yes, sending diff tarball to containerd seems costly here. For containerd worker mode, I think we should use the diff service of containerd instead of our overlayfs-differ.

@tonistiigi
Copy link
Member

tried buffered copy (ktock/buildkit@cc0ffec) for writing to the content store

I don't think the comment to "use io.CopyBuffer" was correct. That does not actually buffer writes but just allows to reuse the internal buffer. So bufio.Writer should be the correct fix here(if this is the issue).

@ktock
Copy link
Collaborator

ktock commented Sep 16, 2021

@tonistiigi Fixed to use bufio.Writer (ktock@aad04dc) but the build in the above still slow in containerd worker ("exporting layers" took 6.5s).
However when I use patched containerd that uses overlayfs differ in diff service (ktock/containerd@9bb1fc1), no regression occurred ("exporting layers" took 5.6s). So the bottleneck seems on the communication path between containerd and buildkitd.

I think we should fix containerd worker not to use client-side differ implementation (must be done before v0.10.0, draft PR at #2366) and should port our overlayfs differ to containerd's differ service (can be long-term work). WDYT?

@tonistiigi
Copy link
Member

If it is just 5.6 vs 6.5 then could be just grpc overhead indeed.

@aaronlehmann can you confirm that you were using containerd worker as well?

@aaronlehmann
Copy link
Collaborator Author

Yes, I am using the containerd worker.

I'm not the one that originally set this up, so I'd be curious about the pros/cons of containerd vs. OCI and whether it would be better for me to switch.

@tonistiigi
Copy link
Member

so I'd be curious about the pros/cons of containerd vs. OCI and whether it would be better for me to switch.

If you are already using containerd outside buildkit then with containerd worker you can load images into or from containerd imagestore and share the blob storage with other components. Otherwise, there are no benefits and OCI is generally probably bit faster because there is no extra grpc layer.

Note for the differ, the grpc overhead shouldn't apply because there is a special diff API in containerd and slow blob creation happens internally in daemon. But with this change we would compute the diff directly in buildkit and then send the blob over grpc to containerd, so now there was much more traffic going to the grpc api.

@tonistiigi
Copy link
Member

@ktock So did I understand correctly that adding bufio, while it didn't make the diff as fast as oci worker or containerd internal differ it still made it much faster. Eg. now it was 10-20% slower while previously it was 3-6 times slower.

@ktock
Copy link
Collaborator

ktock commented Sep 17, 2021

@tonistiigi

So did I understand correctly that adding bufio, while it didn't make the diff as fast as oci worker or containerd internal differ it still made it much faster. Eg. now it was 10-20% slower while previously it was 3-6 times slower.

Yes.

@tonistiigi tonistiigi changed the title [master] Large performance regression exporting layers [master] Large performance regression exporting layers with containerd worker Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants