Skip to content

daemon: don't adjust oom-score if score is 0#41527

Merged
tiborvass merged 1 commit intomoby:masterfrom
thaJeztah:no_oom_score_adj
Oct 15, 2020
Merged

daemon: don't adjust oom-score if score is 0#41527
tiborvass merged 1 commit intomoby:masterfrom
thaJeztah:no_oom_score_adj

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Oct 5, 2020

relates to / addresses docker/for-linux#1120

This patch makes two changes if --oom-score-adj is set to 0

  • do not adjust the oom-score-adjust cgroup for dockerd
  • do not set the hard-coded -999 score for containerd if
    containerd is running as child process

Before this change:

oom-score-adj dockerd containerd as child-process
- -500 -500 (same as dockerd)
-100 -100 -100 (same as dockerd)
0 0 -999 (hard-coded default)

With this change:

oom-score-adj dockerd containerd as child-process
- -500 -500 (same as dockerd)
-100 -100 -100 (same as dockerd)
0 not adjusted not adjusted

- Description for the changelog

Do not adjust the OOM-score for the daemon and containerd if --oom-score-adj is 0

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added this to the 20.03.0 milestone Oct 5, 2020
@thaJeztah

This comment has been minimized.

@cpuguy83
Copy link
Copy Markdown
Member

cpuguy83 commented Oct 5, 2020

I would argue that we should take this out and handle it in the process supervisor.... but we could also just not error out here.

This patch makes two changes if --oom-score-adj is set to 0

- do not adjust the oom-score-adjust cgroup for dockerd
- do not set the hard-coded -999 score for containerd if
  containerd is running as child process

Before this change:

oom-score-adj | dockerd       | containerd as child-process
--------------|---------------|----------------------------
-             | -500          | -500 (same as dockerd)
-100          | -100          | -100 (same as dockerd)
 0            |  0            | -999 (hard-coded default)

With this change:

oom-score-adj | dockerd       | containerd as child-process
--------------|---------------|----------------------------
-             | -500          | -500 (same as dockerd)
-100          | -100          | -100 (same as dockerd)
0             | not adjusted  | not adjusted

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member Author

@cpuguy83 yes, I was already preparing a PR to remove the default -500 and let systemd set the OOM-score adjust.

Oh! I had a more descriptive commit-message that I didn't push; let me do so

@thaJeztah
Copy link
Copy Markdown
Member Author

Follow up (marked as WIP, for now) in #41528

Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants