Skip to content

Don't fatal on epoll wait#2471

Merged
stevvooe merged 1 commit intocontainerd:masterfrom
crosbymichael:fatal
Jul 23, 2018
Merged

Don't fatal on epoll wait#2471
stevvooe merged 1 commit intocontainerd:masterfrom
crosbymichael:fatal

Conversation

@crosbymichael
Copy link
Copy Markdown
Member

@crosbymichael crosbymichael commented Jul 17, 2018

This removes a log fatal on epoll wait for OOM events.

Signed-off-by: Michael Crosby [email protected]

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Jul 17, 2018 •
edited
Loading

Was this used as an exit condition before or was the loop never supposed to exit? Curious where this was being seen previously, and if the error could be repeated forever.

dmcgowan
dmcgowan reviewed Jul 17, 2018
View reviewed changes
Comment thread metrics/cgroups/oom.go Outdated
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan Jul 17, 2018

Choose a reason for hiding this comment

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

This could be simplified to only have one continue statement

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 18, 2018 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2471   +/-   ##
=======================================
  Coverage   44.73%   44.73%           
=======================================
  Files          93       93           
  Lines        9490     9490           
=======================================
  Hits         4245     4245           
  Misses       4562     4562           
  Partials      683      683
Flag Coverage Δ
#linux 48.93% <ø> (ø) ⬆️
#windows 41.03% <ø> (ø) ⬆️

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 94cfce6...9743ff2. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member Author

crosbymichael commented Jul 18, 2018

@dmcgowan updated

@stevvooe
Copy link
Copy Markdown
Member

stevvooe commented Jul 18, 2018

I may have missed context here but what error is returned from epoll wait if the fd gets closed?

@crosbymichael
Copy link
Copy Markdown
Member Author

crosbymichael commented Jul 18, 2018

it returns an EBADF. I added an update to reopen epoll if we get an EBADF, if we get it more than 5 times, then return

@crosbymichael
Don't fatal on epoll wait
9743ff2 
This removes a log fatal on epoll wait for OOM events.

Signed-off-by: Michael Crosby <[email protected]>
@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Jul 18, 2018

LGTM

1 similar comment
@stevvooe
Copy link
Copy Markdown
Member

stevvooe commented Jul 23, 2018

LGTM

@stevvooe stevvooe merged commit a88b631 into containerd:master Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@crosbymichael @dmcgowan @codecov-io @stevvooe