Skip to content

Fix a bug in shim log on Windows that can cause 100% CPU utilization#3050

Merged
dmcgowan merged 1 commit intocontainerd:masterfrom
jterry75:fix_cpu_cap
Feb 28, 2019
Merged

Fix a bug in shim log on Windows that can cause 100% CPU utilization#3050
dmcgowan merged 1 commit intocontainerd:masterfrom
jterry75:fix_cpu_cap

Conversation

@jterry75
Copy link
Copy Markdown
Contributor

With the change to unified shims (ie: 1 shim per multiple tasks) the shimLog
on Windows for the 2nd-Nth worload containers will not have an associated
named pipe listener.

Due to a subtle bug in errors.Wrap passing a nil error we would unblock the
disconnected listener and return 0 byte successfull reads which would cause go
to continually read and cap the CPU.

Signed-off-by: Justin Terry (VM) [email protected]

With the change to unified shims (ie: 1 shim per multiple tasks) the shimLog
on Windows for the 2nd-Nth worload containers will not have an associated
named pipe listener.

Due to a subtle bug in errors.Wrap passing a nil error we would unblock the
disconnected listener and return 0 byte successfull reads which would cause go
to continually read and cap the CPU.

Signed-off-by: Justin Terry (VM) <[email protected]>
@jterry75
Copy link
Copy Markdown
Contributor Author

@jhowardmsft, @kevpar - FYI

Copy link
Copy Markdown

@lowenna lowenna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Fun bug!!!

@codecov-io
Copy link
Copy Markdown

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3050   +/-   ##
=======================================
  Coverage   43.87%   43.87%           
=======================================
  Files         102      102           
  Lines       10903    10903           
=======================================
  Hits         4784     4784           
  Misses       5384     5384           
  Partials      735      735
Flag Coverage Δ
#linux 47.48% <ø> (ø) ⬆️
#windows 41.08% <ø> (ø) ⬆️

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 e72ad44...828f6eb. Read the comment docs.

@dmcgowan
Copy link
Copy Markdown
Member

LGTM

Does this solve the 100% CPU issue mentioned earlier?

@dmcgowan dmcgowan merged commit c24a743 into containerd:master Feb 28, 2019
@lowenna
Copy link
Copy Markdown

lowenna commented Feb 28, 2019

Yes. We are pretty certain this is the same root cause

@jterry75 jterry75 deleted the fix_cpu_cap branch February 28, 2019 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants