Skip to content

Add OomScoreAdj#16277

Merged
icecrime merged 1 commit intomoby:masterfrom
runcom:add-oom-score-adj
Dec 2, 2015
Merged

Add OomScoreAdj#16277
icecrime merged 1 commit intomoby:masterfrom
runcom:add-oom-score-adj

Conversation

@runcom
Copy link
Copy Markdown
Member

@runcom runcom commented Sep 14, 2015

libcontainer v0.0.4 introduces setting /proc/self/oom_score_adj to
better tune oom killing preferences for container process. This patch
simply integrates OomScoreAdj libcontainer's config option and adjust
the cli with this new option.

close #13116

TODOs:

  • Docs
  • Tests

Signed-off-by: Antonio Murdaca [email protected]

@runcom runcom changed the title Add --oom-score-adj [WIP] Add --oom-score-adj Sep 14, 2015
@runcom runcom force-pushed the add-oom-score-adj branch 2 times, most recently from 427de1e to 2bc25ae Compare September 14, 2015 11:15
@runcom runcom changed the title [WIP] Add --oom-score-adj [WIP] Add OomScoreAdj Sep 14, 2015
@runcom runcom force-pushed the add-oom-score-adj branch 2 times, most recently from 91e1cc3 to 0a49ad8 Compare September 14, 2015 16:23
RichardScothern
RichardScothern reviewed Sep 14, 2015
View reviewed changes
Comment thread runconfig/parse.go Outdated
Copy link
Copy Markdown
Contributor

@RichardScothern RichardScothern Sep 14, 2015

Choose a reason for hiding this comment

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

Should this description contain the accepted range (-1000, 1000)?

@runcom runcom force-pushed the add-oom-score-adj branch 2 times, most recently from acc21cc to 909e519 Compare September 14, 2015 19:01
@runcom runcom force-pushed the add-oom-score-adj branch 9 times, most recently from 899b7af to dfeacbf Compare September 18, 2015 18:23
@jessfraz jessfraz changed the title [WIP] Add OomScoreAdj Add OomScoreAdj Oct 8, 2015
@icecrime
Copy link
Copy Markdown
Contributor

icecrime commented Oct 8, 2015

Design OK

@icecrime
Copy link
Copy Markdown
Contributor

icecrime commented Oct 8, 2015

Ping @runcom for rebase before code review!

@runcom runcom force-pushed the add-oom-score-adj branch from dfeacbf to 4ff44c7 Compare October 8, 2015 19:01
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Oct 8, 2015

@icecrime rebased

@vdemeester
Copy link
Copy Markdown
Member

vdemeester commented Oct 8, 2015

@runcom looks like the vet is not happy 😅

thaJeztah
thaJeztah reviewed Nov 26, 2015
View reviewed changes
Comment thread contrib/completion/zsh/_docker Outdated
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Nov 26, 2015

Choose a reason for hiding this comment

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

Can you use the same text here as well?

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Nov 26, 2015

@moxiegirl I see we're inconsistent with describing the options; Tunes ... something v.s. Tune ... something. I think we should pick one direction, so either "(this option) does something", or "(use this option to) do something"; wdyt?

@moxiegirl
Copy link
Copy Markdown
Contributor

moxiegirl commented Nov 29, 2015

@thaJeztah Yes, I noticed that. I think that is a good idea but the problem is beyond the scope of this PR. It is really something we should do in another PR. WDYT?

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Nov 29, 2015

@moxiegirl I think so far we used Tune... but your suggestions here were Tunes.., that was my reason for mentioning it. But if there are other inconsistencies, I'm perfectly fine changing it in a separate PR

@moxiegirl
Copy link
Copy Markdown
Contributor

moxiegirl commented Nov 29, 2015

Ah, I see. Yes, you are right Tune is more consistent with the other options.

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Nov 30, 2015

@moxiegirl @thaJeztah updated (I'll leave what's not consistent to a follow up PR as you agreed on)

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Nov 30, 2015

@runcom looks like I overlooked that changes to the man-pages are missing, can you add the flag to the man-pages for create and run? 😊

@runcom
Copy link
Copy Markdown
Member Author

runcom commented Nov 30, 2015

@thaJeztah added

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Nov 30, 2015

Thanks! And thanks for being patient with us 👍

LGTM

ping @moxiegirl for a final look

@runcom
Add OomScoreAdj to configure container oom killer preferences
d3af7f2 
libcontainer v0.0.4 introduces setting `/proc/self/oom_score_adj` to
better tune oom killing preferences for container process. This patch
simply integrates OomScoreAdj libcontainer's config option and adjust
the cli with this new option.

Signed-off-by: Antonio Murdaca <[email protected]>
Signed-off-by: Antonio Murdaca <[email protected]>
@runcom
Copy link
Copy Markdown
Member Author

runcom commented Dec 2, 2015

ping @moxiegirl O:)

@moxiegirl
Copy link
Copy Markdown
Contributor

moxiegirl commented Dec 2, 2015

@runcom LGTM to you! :-D Thank you. @thaJeztah for the merge

icecrime pushed a commit that referenced this pull request Dec 2, 2015
Merge pull request #16277 from runcom/add-oom-score-adj
8f1f53f 
Add OomScoreAdj
@icecrime icecrime merged commit 8f1f53f into moby:master Dec 2, 2015
@runcom runcom deleted the add-oom-score-adj branch December 2, 2015 19:50
@thaJeztah thaJeztah added this to the 1.10.0 milestone Jan 15, 2016
This was referenced Jan 25, 2016
Closed
Closed
@resouer resouer mentioned this pull request Jan 27, 2016
Merged
@awasilyev awasilyev mentioned this pull request Apr 7, 2016
Closed
@Knetic Knetic mentioned this pull request Jun 8, 2016
Closed
@thaJeztah thaJeztah mentioned this pull request Mar 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

impact/api impact/changelog impact/cli status/4-merge

Projects

None yet

Milestone

1.10.0

Development

Successfully merging this pull request may close these issues.

Support setting OOM score