Improve image pull performance from http 1.1 container registries by amrmahdi · Pull Request #4653 · containerd/containerd

amrmahdi · 2020-10-26T05:06:43Z

Private registries that does not support http 2.0 such as Azure Container Registry streams back content in a max of 16KB chunks (max TLS record size). The small chunks introduce an overhead when copying the layers to the content store sine each chunk incurs the overhead of grpc message that has to be sent to the content store.

This change reduces this overhead by buffering the chunks into 1MB chunks and only then writes a message to the content store.

Below is a per comparsion between the 2 approaches using a couple of large images that are being pulled from the docker hub (http 2.0) and a private Azure CR (http 1.1) in seconds.

image	Buffered copy	master
docker.io/pytorch/pytorch:latest	55.63	58.33
docker.io/nvidia/cuda:latest	72.05	75.98
containerdpulltest.azurecr.io/pytorch/pytorch:latest	61.45	77.1
containerdpulltest.azurecr.io/nvidia/cuda:latest	77.13	85.47

Test setup:

Cloud: Azure
OS: Linux
VM Size: Standard_DS13_v2
Disk Size: 128GB
VM location: West US
Private ACR Location: West US

Signed-off-by: Amr Mahdi [email protected]

Private registries that does not support http 2.0 such as Azure Container Registry streams back content in a max of 16KB chunks (max TLS record size). The small chunks introduce an overhead when copying the layers to the content store sine each chunk incurs the overhead of grpc message that has to be sent to the content store. This change reduces this overhead by buffering the chunks into 1MB chunks and only then writes a message to the content store. Below is a per comparsion between the 2 approaches using a couple of large images that are being pulled from the docker hub (http 2.0) and a private Azure CR (http 1.1) in seconds. image | Buffered copy | master ------- |---------------|---------- docker.io/pytorch/pytorch:latest | 55.63 | 58.33 docker.io/nvidia/cuda:latest | 72.05 | 75.98 containerdpulltest.azurecr.io/pytorch/pytorch:latest | 61.45 | 77.1 containerdpulltest.azurecr.io/nvidia/cuda:latest | 77.13 | 85.47 Signed-off-by: Amr Mahdi <[email protected]>

k8s-ci-robot · 2020-10-26T05:06:52Z

Hi @amrmahdi. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

theopenlab-ci · 2020-10-26T05:13:54Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 52s (non-voting)

AkihiroSuda · 2020-10-26T07:46:00Z

/ok-to-test

cpuguy83 · 2020-10-27T21:01:51Z

Looks like you are missing DCO on the 2nd commit.

Signed-off-by: Amr Mahdi <[email protected]>

theopenlab-ci · 2020-10-28T05:57:47Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 55s (non-voting)

cpuguy83

LGTM

mikebrow

couple comments

Signed-off-by: Amr Mahdi <[email protected]>

theopenlab-ci · 2020-11-03T04:33:18Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 59s (non-voting)

cpuguy83 · 2020-11-12T01:10:03Z

@mikebrow LGTY?

mikebrow

LGTM

cpuguy83

LGTM

kzys · 2020-11-16T18:49:36Z

Maintainers can merge this PR since there are two approvals.

estesp

LGTM

…t-transfer Improve image pull performance from http 1.1 container registries (cherry picked from commit cc3785c) Signed-off-by: Amr Mahdi <[email protected]>

- Update to containerd 1.4.2 - Update Golang runtime to 1.15.5 Upstream containerd 1.4.2 release notes: https://github.com/containerd/containerd/releases/tag/v1.4.2 Welcome to the v1.4.2 release of containerd! ------------------------------------------------------ The second patch release for containerd 1.4 includes multiple minor fixes and updates. Notable Updates - Fix bug limiting the number of layers by default containerd/cri#1602 - Fix selinux shared memory issue by relabeling /dev/shm containerd/cri#1605 - Fix unknown state preventing removal of containers containerd/containerd#4656 - Fix nil pointer error when restoring checkpoint containerd/containerd#4754 - Improve image pull performance when using HTTP 1.1 containerd/containerd#4653 - Update default seccomp profile for pidfd containerd/containerd#4730 - Update Go to 1.15 Windows - Fix integer overflow on Windows containerd/containerd#4589 - Fix lcow snapshotter to read trailing tar data containerd/containerd#4628 Signed-off-by: Sebastiaan van Stijn <[email protected]>

NOTE: the Dockerfile currently uses a single version of Golang for all stages. This means that currently, all binaries are built with Go 1.13.x, including the containerd binary; upstream containerd switched to use Go 1.15. full diff: containerd/containerd@v1.4.1...v1.4.2 Release notes: Welcome to the v1.4.2 release of containerd! ------------------------------------------------------ The second patch release for containerd 1.4 includes multiple minor fixes and updates. Notable Updates - Fix bug limiting the number of layers by default containerd/cri#1602 - Fix selinux shared memory issue by relabeling /dev/shm containerd/cri#1605 - Fix unknown state preventing removal of containers containerd/containerd#4656 - Fix nil pointer error when restoring checkpoint containerd/containerd#4754 - Improve image pull performance when using HTTP 1.1 containerd/containerd#4653 - Update default seccomp profile for pidfd containerd/containerd#4730 - Update Go to 1.15 Windows - Fix integer overflow on Windows containerd/containerd#4589 - Fix lcow snapshotter to read trailing tar data containerd/containerd#4628 Signed-off-by: Sebastiaan van Stijn <[email protected]>

NOTE: the Dockerfile currently uses a single version of Golang for all stages. This means that currently, all binaries are built with Go 1.13.x, including the containerd binary; upstream containerd switched to use Go 1.15. full diff: containerd/containerd@v1.4.1...v1.4.2 Release notes: Welcome to the v1.4.2 release of containerd! ------------------------------------------------------ The second patch release for containerd 1.4 includes multiple minor fixes and updates. Notable Updates - Fix bug limiting the number of layers by default containerd/cri#1602 - Fix selinux shared memory issue by relabeling /dev/shm containerd/cri#1605 - Fix unknown state preventing removal of containers containerd/containerd#4656 - Fix nil pointer error when restoring checkpoint containerd/containerd#4754 - Improve image pull performance when using HTTP 1.1 containerd/containerd#4653 - Update default seccomp profile for pidfd containerd/containerd#4730 - Update Go to 1.15 Windows - Fix integer overflow on Windows containerd/containerd#4589 - Fix lcow snapshotter to read trailing tar data containerd/containerd#4628 Signed-off-by: Sebastiaan van Stijn <[email protected]> Upstream-commit: 703951197c3338631ee0529dd9dd814d16f037f0 Component: engine

- Update to containerd 1.4.2 - Update Golang runtime to 1.15.5 Upstream containerd 1.4.2 release notes: https://github.com/containerd/containerd/releases/tag/v1.4.2 Welcome to the v1.4.2 release of containerd! ------------------------------------------------------ The second patch release for containerd 1.4 includes multiple minor fixes and updates. Notable Updates - Fix bug limiting the number of layers by default containerd/cri#1602 - Fix selinux shared memory issue by relabeling /dev/shm containerd/cri#1605 - Fix unknown state preventing removal of containers containerd/containerd#4656 - Fix nil pointer error when restoring checkpoint containerd/containerd#4754 - Improve image pull performance when using HTTP 1.1 containerd/containerd#4653 - Update default seccomp profile for pidfd containerd/containerd#4730 - Update Go to 1.15 Windows - Fix integer overflow on Windows containerd/containerd#4589 - Fix lcow snapshotter to read trailing tar data containerd/containerd#4628 Signed-off-by: Sebastiaan van Stijn <[email protected]>

NOTE: the Dockerfile currently uses a single version of Golang for all stages. This means that currently, all binaries are built with Go 1.13.x, including the containerd binary; upstream containerd switched to use Go 1.15. full diff: containerd/containerd@v1.4.1...v1.4.2 Release notes: Welcome to the v1.4.2 release of containerd! ------------------------------------------------------ The second patch release for containerd 1.4 includes multiple minor fixes and updates. Notable Updates - Fix bug limiting the number of layers by default containerd/cri#1602 - Fix selinux shared memory issue by relabeling /dev/shm containerd/cri#1605 - Fix unknown state preventing removal of containers containerd/containerd#4656 - Fix nil pointer error when restoring checkpoint containerd/containerd#4754 - Improve image pull performance when using HTTP 1.1 containerd/containerd#4653 - Update default seccomp profile for pidfd containerd/containerd#4730 - Update Go to 1.15 Windows - Fix integer overflow on Windows containerd/containerd#4589 - Fix lcow snapshotter to read trailing tar data containerd/containerd#4628 Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit 7039511) Signed-off-by: Sebastiaan van Stijn <[email protected]>

k8s-ci-robot added the needs-ok-to-test label Oct 26, 2020

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Oct 26, 2020

cpuguy83 reviewed Oct 26, 2020

View reviewed changes

Comment thread content/helpers.go

amrmahdi requested a review from cpuguy83 October 27, 2020 06:35

replicate io.Copy optimizations

f6834d4

Signed-off-by: Amr Mahdi <[email protected]>

amrmahdi force-pushed the amrh/optimize-content-transfer branch from a8a70ea to f6834d4 Compare October 28, 2020 05:50

cpuguy83 approved these changes Oct 28, 2020

View reviewed changes

mikebrow reviewed Nov 2, 2020

View reviewed changes

Comment thread content/helpers.go

Comment thread content/helpers.go

Comment thread content/helpers.go

amrmahdi requested a review from mikebrow November 2, 2020 20:01

Add comments clarifying copyWithBuffer implementation

b81917e

Signed-off-by: Amr Mahdi <[email protected]>

amrmahdi force-pushed the amrh/optimize-content-transfer branch from ec7cd79 to b81917e Compare November 3, 2020 04:25

mikebrow approved these changes Nov 12, 2020

View reviewed changes

cpuguy83 approved these changes Nov 16, 2020

View reviewed changes

estesp approved these changes Nov 16, 2020

View reviewed changes

estesp merged commit cc3785c into containerd:master Nov 16, 2020

cpuguy83 added the cherry-pick/1.4.x label Nov 18, 2020

fuweid mentioned this pull request Nov 18, 2020

[release/1.4] Improve image pull performance from http 1.1 container registries #4751

Merged

fuweid added the cherry-picked/1.4.x PR commits are cherry picked into the release/1.4 branch label Nov 18, 2020

fuweid removed the cherry-pick/1.4.x label Nov 18, 2020

thaJeztah mentioned this pull request Nov 26, 2020

release 1.4.3 (CVE-2020-15257) docker/containerd-packaging#204

Merged

thaJeztah mentioned this pull request Nov 26, 2020

update containerd binary to v1.4.2 moby/moby#41718

Merged

Conversation

amrmahdi commented Oct 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test setup:

Uh oh!

k8s-ci-robot commented Oct 26, 2020

Uh oh!

theopenlab-ci Bot commented Oct 26, 2020

Uh oh!

AkihiroSuda commented Oct 26, 2020

Uh oh!

Uh oh!

cpuguy83 commented Oct 27, 2020

Uh oh!

theopenlab-ci Bot commented Oct 28, 2020

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

theopenlab-ci Bot commented Nov 3, 2020

Uh oh!

cpuguy83 commented Nov 12, 2020

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

kzys commented Nov 16, 2020

Uh oh!

estesp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

amrmahdi commented Oct 26, 2020 •

edited

Loading