Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime-config-linux: Add info to clarify oom_score_adj (carrying #236) #292

Merged
merged 2 commits into from
Jan 6, 2016

Conversation

vbatts
Copy link
Member

@vbatts vbatts commented Jan 5, 2016

This is carrying #236, but deferring the description to the linux docs.

@@ -180,7 +180,13 @@ For more information, see [the memory cgroup man page](https://www.kernel.org/do

#### Set oom_score_adj

More information on `oom_score_adj` available [here](https://www.kernel.org/doc/Documentation/filesystems/proc.txt).
oom_score_adj works with disableOOMKiller.
If out-of-memory killer disabled("disableOOMKiller": true), the value of oom_score_adj is useless and will be omitted.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@vishh vishh self-assigned this Jan 5, 2016
@@ -180,7 +180,14 @@ For more information, see [the memory cgroup man page](https://www.kernel.org/do

#### Set oom_score_adj

More information on `oom_score_adj` available [here](https://www.kernel.org/doc/Documentation/filesystems/proc.txt).
oom_score_adj sets heuristic regarding how the process is evaluated by the kernel during memory pressure.
Copy link
Contributor

Choose a reason for hiding this comment

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

“oom_score_adj” should probaby be “oom_score_adj” or “oom_score_adj”. And maybe reword to:

oom_score_adj adjusts the kernel's process evaluation during memory pressure.

Copy link
Member Author

Choose a reason for hiding this comment

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

but it is per process

Copy link
Member Author

Choose a reason for hiding this comment

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

and i agree, re backticks

Copy link
Contributor

Choose a reason for hiding this comment

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

On Tue, Jan 05, 2016 at 12:47:42PM -0800, Vincent Batts wrote:

but it is per process

Right. I just think “sets heuristic regarding how the process is
evaluated by the kernel” could be condensed without losing
information. How about something closer to the kernel's §3.1 lead in
1:

oom_score_adj adjusts the OOM-killer badness heuristic for this
process.

?

@wking
Copy link
Contributor

wking commented Jan 5, 2016

I left a few inline copy-edit suggestions for 1bfadf6, but the thrust
looks good to me. While we're cleaning up the docs for this setting,
we may want to consider clarifying the runtime action if the setting
is unset or null 1.

@hqhq
Copy link
Contributor

hqhq commented Jan 6, 2016

LGTM

@philips
Copy link
Contributor

philips commented Jan 6, 2016

lgtm

@vishh
Copy link
Contributor

vishh commented Jan 6, 2016

LGTM

vishh added a commit that referenced this pull request Jan 6, 2016
runtime-config-linux: Add info to clarify oom_score_adj (carrying #236)
@vishh vishh merged commit 6a6ba67 into opencontainers:master Jan 6, 2016
@wking wking mentioned this pull request Jan 13, 2016
@vbatts vbatts deleted the tiesheng_oomscoreadj branch April 12, 2016 03:53
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request May 9, 2017
The only discussion related to this is in [1,2], where the
relationship between oomScoreAdj and disableOOMKiller is raised. But
since 429f936 (Adding cgroups path to the Spec, 2015-09-02, opencontainers#137)
resources has been tied to cgroups, and oomScoreAdj is not about
cgroups.  For example, we currently have (in config-linux.md):

  You can configure a container's cgroups via the resources field of
  the Linux configuration.

I suggested we move the property from linux.resources.oomScoreAdj to
linux.oomScoreAdj so config authors and runtimes don't have to worry
about what cgroupsPath means if the only entry in resources is
oomScoreAdj.  Michael responded with [4]:

  If anything it should probably go on the process

So that's what this commit does.

[1]: opencontainers#236
[2]: opencontainers#292
[3]: opencontainers#137
[4]: opencontainers#782 (comment)

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request May 9, 2017
The only discussion related to this is in [1,2], where the
relationship between oomScoreAdj and disableOOMKiller is raised. But
since 429f936 (Adding cgroups path to the Spec, 2015-09-02, opencontainers#137)
resources has been tied to cgroups, and oomScoreAdj is not about
cgroups.  For example, we currently have (in config-linux.md):

  You can configure a container's cgroups via the resources field of
  the Linux configuration.

I suggested we move the property from linux.resources.oomScoreAdj to
linux.oomScoreAdj so config authors and runtimes don't have to worry
about what cgroupsPath means if the only entry in resources is
oomScoreAdj.  Michael responded with [4]:

  If anything it should probably go on the process

So that's what this commit does.

I've gone with the four-space indents here to keep Pandoc happy (see
7795661 (runtime.md: Fix sub-bullet indentation, 2016-06-08, opencontainers#495),
but have left the existing entries in this list unchanged to reduce
churn.

[1]: opencontainers#236
[2]: opencontainers#292
[3]: opencontainers#137
[4]: opencontainers#782 (comment)

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request May 9, 2017
The only discussion related to this is in [1,2], where the
relationship between oomScoreAdj and disableOOMKiller is raised. But
since 429f936 (Adding cgroups path to the Spec, 2015-09-02, opencontainers#137)
resources has been tied to cgroups, and oomScoreAdj is not about
cgroups.  For example, we currently have (in config-linux.md):

  You can configure a container's cgroups via the resources field of
  the Linux configuration.

I suggested we move the property from linux.resources.oomScoreAdj to
linux.oomScoreAdj so config authors and runtimes don't have to worry
about what cgroupsPath means if the only entry in resources is
oomScoreAdj.  Michael responded with [4]:

  If anything it should probably go on the process

So that's what this commit does.

I've gone with the four-space indents here to keep Pandoc happy (see
7795661 (runtime.md: Fix sub-bullet indentation, 2016-06-08, opencontainers#495),
but have left the existing entries in this list unchanged to reduce
churn.

[1]: opencontainers#236
[2]: opencontainers#292
[3]: opencontainers#137
[4]: opencontainers#782 (comment)

Signed-off-by: W. Trevor King <[email protected]>
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.

6 participants