Use cached state instead of `runc state`. by Random-Liu · Pull Request #3711 · containerd/containerd

Random-Liu · 2019-10-03T04:57:49Z

See kubernetes/kubernetes#82440 and moby/moby#39102

I think we should cherry-pick this into 1.2 and 1.3.

In Kubernetes, we observed that exec in containerd is fair expensive. With Docker 18.09, for each exec process, 8 runc state processes will be spawned.

The followings are results of sudo strace -f -p SHIM_PID -e execve when we simply run a crictl exec CONTAINER_ID echo hello.

Containerd with containerd-shim (I believe most people are still using this):

$ cat result | grep execve
[pid 229037] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc000059000 /* 6 vars */) = 0
[pid 229043] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc0001393c0 /* 6 vars */) = 0
[pid 229049] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc000059080 /* 6 vars */) = 0
[pid 229055] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "exec", "--process", "/tmp/runc-process270233118", "--detach", "--pid-file", "/run/containerd/io.containerd.ru"..., "174c9870fbd569c79d7e8d706323134a"...], 0xc000059140 /* 6 vars */) = 0
[pid 229062] execve("/proc/self/exe", ["runc", "init"], 0xc000058500 /* 6 vars */) = 0
[pid 229066] execve("/bin/echo", ["echo", "hello"], 0xc00007d0c0 /* 3 vars */ <unfinished ...>
[pid 229066] <... execve resumed> )     = 0
[pid 229073] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc000059340 /* 6 vars */) = 0
[pid 229074] execve("/usr/local/bin/containerd", ["/usr/local/bin/containerd", "--address", "/run/containerd/containerd.sock", "publish", "--topic", "/tasks/exit", "--namespace", "k8s.io"], 0xc0002d0540 /* 6 vars */) = 0
[pid 229080] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc0002d0a40 /* 6 vars */) = 0
[pid 229091] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc0000593c0 /* 6 vars */) = 0
[pid 229099] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc000059480 /* 6 vars */) = 0
[pid 229105] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc0002d17c0 /* 6 vars */) = 0
[pid 229111] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "174c9870fbd569c79d7e8d706323134a"...], 0xc000059940 /* 6 vars */) = 0

Containerd with containerd-shim-runc-v1 (shim v2 implementation has less unnecessary runc state, because those unnecessary runc state are introduced by containerd-shim specific code https://github.com/containerd/containerd/blob/v1.3.0/runtime/v1/linux/task.go#L318):

$ cat result | grep execve
[pid 225273] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc.v1/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "9d7dfbb4ce08f1ed732ad9683d6ab09b"...], 0xc000290040 /* 7 vars */) = 0
[pid 225280] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc.v1/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "exec", "--process", "/tmp/runc-process475686972", "--detach", "--pid-file", "/run/containerd/io.containerd.ru"..., "9d7dfbb4ce08f1ed732ad9683d6ab09b"...], 0xc000290380 /* 7 vars */) = 0
[pid 225286] execve("/proc/self/exe", ["runc", "init"], 0xc00005a200 /* 6 vars */) = 0
[pid 225290] execve("/bin/echo", ["echo", "hello"], 0xc00007d0c0 /* 3 vars */ <unfinished ...>
[pid 225290] <... execve resumed> )     = 0
[pid 225297] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc.v1/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "9d7dfbb4ce08f1ed732ad9683d6ab09b"...], 0xc000290880 /* 7 vars */) = 0
[pid 225303] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc.v1/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "state", "9d7dfbb4ce08f1ed732ad9683d6ab09b"...], 0xc000290980 /* 7 vars */) = 0

Containerd with containerd-shim with this change (no runc state anymore):

$ cat result | grep execve
[pid 220550] execve("/usr/local/sbin/runc", ["runc", "--root", "/run/containerd/runc/k8s.io", "--log", "/run/containerd/io.containerd.ru"..., "--log-format", "json", "exec", "--process", "/tmp/runc-process002244826", "--detach", "--pid-file", "/run/containerd/io.containerd.ru"..., "1821a71464ad289dc60f6fd6a2d1a2cd"...], 0xc00007e200 /* 6 vars */) = 0
[pid 220556] execve("/proc/self/exe", ["runc", "init"], 0xc000058300 /* 6 vars */) = 0
[pid 220560] execve("/bin/echo", ["echo", "hello"], 0xc00007d0c0 /* 3 vars */ <unfinished ...>
[pid 220560] <... execve resumed> )     = 0
[pid 220568] execve("/usr/local/bin/containerd", ["/usr/local/bin/containerd", "--address", "/run/containerd/containerd.sock", "publish", "--topic", "/tasks/exit", "--namespace", "k8s.io"], 0xc00007f000 /* 6 vars */) = 0

Signed-off-by: Lantao Liu lantaol@google.com

Random-Liu · 2019-10-03T05:06:28Z

@crosbymichael Do you see any problem with using the cached state instead of invoking runc state?

theopenlab-ci · 2019-10-03T05:07:41Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 9m 06s (non-voting)

codecov-io · 2019-10-03T05:20:38Z

Codecov Report

Merging #3711 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #3711   +/-   ##
======================================
  Coverage    42.1%   42.1%           
======================================
  Files         129     129           
  Lines       14307   14307           
======================================
  Hits         6024    6024           
  Misses       7383    7383           
  Partials      900     900

Flag	Coverage Δ
#linux	`45.61% <ø> (ø)`	⬆️
#windows	`37.05% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a83ae30...d3dc6cb. Read the comment docs.

crosbymichael · 2019-10-03T16:18:16Z

LGTM

Random-Liu · 2019-10-03T17:49:54Z

@crosbymichael I updated the pausing implementation a little bit. In the previous implementation, it is possible that the container goes from pausing to running state after pausing boolean is reset but not transition into paused state yet.

And let's test this change in HEAD for several days before cherry-picking. The Status function will need the shim lock after this change. I don't think it will cause any issue, but let's see.

Signed-off-by: Lantao Liu <lantaol@google.com>

theopenlab-ci · 2019-10-03T18:01:53Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 7m 45s (non-voting)

crosbymichael · 2019-10-03T18:52:30Z

Yes, it needs to be in HEAD to make sure that this does not introduce any races or incorrect state changes

jterry75 · 2019-10-03T20:25:40Z

pkg/process/utils.go

 	s.Unlock()
 }

+type atomicBool int32


Why is this not locked by p.mu yes this a atomic so its thread safe but it still enables the race conditions between a status and pause call.

Because pausing only makes sense outside of p.mu. Within the lock, there are only running and paused state.

Hmm. I'll take your word for it :)

Random-Liu · 2019-10-04T00:58:39Z

Can we get this in sooner so that we can start soaking?

This is affecting both Kubernetes and Docker users now. :)

crosbymichael · 2019-10-04T15:20:08Z

LGTM

mxpv

LGTM

containerd 1.3.2 Welcome to the v1.3.2 release of containerd! The second patch release for `containerd` 1.3 includes a fix for a race condition related to the reported pid on exit when called from Docker. ### Runtime * Fix containerd pid race condition [containerd#3857](containerd#3857) * Use cached process state to reduce exec cost [containerd#3711](containerd#3711) ### CRI * Added `insecure_skip_verify` option in the registry tls config to allow skipping registry certificate verification [containerd#3847](containerd#3847) Please try out the release binaries and report any issues at https://github.com/containerd/containerd/issues. ### Contributors * Lantao Liu * Derek McGowan * Michael Crosby * Phil Estes * Maksym Pavlenko ### Changes * [`ff48f57fc8`](containerd@ff48f57) Merge pull request [containerd#3866](containerd#3866) from dmcgowan/prepare-1.3.2 * [`99005c2647`](containerd@99005c2) Add release notes for v1.3.2 * [`e987ea3cac`](containerd@e987ea3) Merge pull request [containerd#3864](containerd#3864) from Random-Liu/update-cri-release-1.3 * [`8bbd71e560`](containerd@8bbd71e) Update cri to b1bef15fbeb6c6f0569b67322acfa74ca3597755. * [`070c27205c`](containerd@070c272) Merge pull request [containerd#3863](containerd#3863) from Random-Liu/cherrypick-#3857-release-1.3 * [`306d6d4b55`](containerd@306d6d4) Fix container pid. * [`5028701f1a`](containerd@5028701) Merge pull request [containerd#3753](containerd#3753) from thaJeztah/1.3_backport_avoid_unnecessary_runc_state * [`e3d2b608cc`](containerd@e3d2b60) Merge pull request [containerd#3855](containerd#3855) from fuweid/cp-3853 * [`04fbb97ad0`](containerd@04fbb97) Fix cleanup error on content client test * [`be24f39454`](containerd@be24f39) Merge pull request [containerd#3840](containerd#3840) from ameyag/1.3_backport_log_file_win * [`333679343b`](containerd@3336793) Add `--log-file` flag for windows service. * [`9abfc70043`](containerd@9abfc70) Use cached state instead of `runc state`. ### Changes from containerd/cri * [`b1bef15f`](containerd/cri@b1bef15) Merge pull request [containerd#1346](containerd/cri#1346) from Random-Liu/cherrypick-#1345-release-1.3 * [`27edfa06`](containerd/cri@27edfa0) Add insecure_skip_verify option. * [`e5dd8053`](containerd/cri@e5dd805) Merge pull request [containerd#1322](containerd/cri#1322) from Random-Liu/cherrypick-#1319-release-1.3 * [`c0dee957`](containerd/cri@c0dee95) Fix containerd build, use `libbtrfs-dev` when available. * [`1fb415d2`](containerd/cri@1fb415d) Merge pull request [containerd#1314](containerd/cri#1314) from Random-Liu/cherrypick-#1312-release-1.3 * [`0973459d`](containerd/cri@0973459) Update based on default xenial distro. * [`a46f6baf`](containerd/cri@a46f6ba) Configure golangci-lint ### Dependency Changes Previous release can be found at [v1.3.1](https://github.com/containerd/containerd/releases/tag/v1.3.1) * **github.com/containerd/cri** 5d49e7e51b43e36a6b9c4386257c7d08c602237f -> b1bef15fbeb6c6f0569b67322acfa74ca3597755 # gpg: directory '/c/Users/juterry/.gnupg' created # gpg: keybox '/c/Users/juterry/.gnupg/pubring.kbx' created # gpg: Signature made Tue Dec 3 11:09:10 2019 PST # gpg: using RSA key 8C7A111C21105794B0E8A27BF58C5D0A4405ACDB # gpg: Can't check signature: No public key

Random-Liu added cherry-pick/1.2.x labels Oct 3, 2019

Random-Liu mentioned this pull request Oct 3, 2019

High system load/CPU utilization with trivial liveness and readiness exec probes kubernetes/kubernetes#82440

Open

Random-Liu force-pushed the avoid-unnecessary-runc-state branch from d3dc6cb to 907914c Compare October 3, 2019 17:49

Random-Liu force-pushed the avoid-unnecessary-runc-state branch from 907914c to 117421f Compare October 3, 2019 17:52

Use cached state instead of runc state.

18be6e3

Signed-off-by: Lantao Liu <lantaol@google.com>

Random-Liu force-pushed the avoid-unnecessary-runc-state branch from 117421f to 18be6e3 Compare October 3, 2019 17:53

Random-Liu mentioned this pull request Oct 3, 2019

Docker healthcheck causes high CPU utilization moby/moby#39102

Open

jterry75 reviewed Oct 3, 2019

View reviewed changes

mxpv approved these changes Oct 4, 2019

View reviewed changes

crosbymichael merged commit 38a0013 into containerd:master Oct 4, 2019

Random-Liu deleted the avoid-unnecessary-runc-state branch October 4, 2019 20:11

This was referenced Oct 4, 2019

Update containerd to a6a0c8b6e36415a151d93d096c1c0af9e0bd7977. containerd/cri#1309

Merged

Reduce runsc state call. google/gvisor-containerd-shim#39

Open

Add unknown state integration test back containerd/cri#1310

Closed

This was referenced Oct 14, 2019

[release/1.3 backport] Use cached state instead of runc state. #3753

Merged

[release/1.2 backport] backport exec fixes #3755

Merged

systemd logs filled with mount unit entries if healtcheck is enabled docker/for-linux#679

Closed

tao12345666333 mentioned this pull request Dec 8, 2019

Update containerd to v1.3.2 moby/moby#40295

Merged

thaJeztah mentioned this pull request Jan 30, 2020

[release/1.3] Sync vendors with containerd 1.3.2 containerd/cri#1391

Merged

thaJeztah mentioned this pull request Jan 30, 2020

[release/1.2] Sync vendors with containerd 1.2.11 containerd/cri#1392

Merged

AkihiroSuda added cherry-picked/1.2.x and removed cherry-pick/1.2.x labels Jun 22, 2020

ianlewis mentioned this pull request Jul 17, 2020

Reduce runsc state calls to the shim google/gvisor#3278

Open

msau42 mentioned this pull request Sep 22, 2020

systemd-run will hang and timeout when numbers of pods and volumes schedule on the node kubernetes/kubernetes#94913

Closed

Conversation

Random-Liu commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Random-Liu commented Oct 3, 2019

Uh oh!

theopenlab-ci bot commented Oct 3, 2019

Uh oh!

codecov-io commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

crosbymichael commented Oct 3, 2019

Uh oh!

Random-Liu commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theopenlab-ci bot commented Oct 3, 2019

Uh oh!

crosbymichael commented Oct 3, 2019

Uh oh!

jterry75 Oct 3, 2019

Choose a reason for hiding this comment

Uh oh!

Random-Liu Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jterry75 Oct 3, 2019

Choose a reason for hiding this comment

Uh oh!

Random-Liu commented Oct 4, 2019

Uh oh!

crosbymichael commented Oct 4, 2019

Uh oh!

mxpv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Random-Liu commented Oct 3, 2019 •

edited

Loading

codecov-io commented Oct 3, 2019 •

edited

Loading

Random-Liu commented Oct 3, 2019 •

edited

Loading

Random-Liu Oct 3, 2019 •

edited

Loading