Limit memory resource usage for nydus daemon via cgroup #415

sctb512 · 2023-03-16T03:25:01Z

Nydus-snapshotter does not have a maximum memory limit by default. This can lead to host invalidity. Now, we put snapshotter and daemon in different cgroup to limit the maximum available memory.

This PR adds two entries to the configuration file.

[cgroup]
# Whether to use separate cgroup for nydusd.
enable = true
# The memory limit for nydusd.slice, which contains all nydusd processes.
# Percentage is supported as well, please ensure it is end with "%".
# The default unit is bytes. Acceptable values include "209715200", "200MiB", "200Mi" and "10%".
memory_limit = ""

imeoer · 2023-03-16T04:12:53Z

pkg/daemon/daemon.go

@@ -559,6 +562,38 @@ func (d *Daemon) ClearVestige() {
 	d.ResetClient()
 }

+func (d *Daemon) AddCgroup(pid int) error {


I think we should implement a cgroup manager and add the daemon to it, instead add the method AddCgroup, DeleteCgroup on *Daemon. One reason is that maybe we need to support containerized snapshotter/nydusd to limit resource usage and process management in the future.

Got it. I will refactor this later.

Agree. We should support resources limitation based on both raw cgroup controller and containers

changweige

Is it based on cgroup v1 or v2?

sctb512 · 2023-03-16T06:32:21Z

Is it based on cgroup v1 or v2?

It is based on cgroup v1.

changweige · 2023-03-17T03:18:56Z

Is it based on cgroup v1 or v2?

It is based on cgroup v1.

Is it possible to depend cgroup v2?

sctb512 · 2023-03-17T03:24:31Z

Is it possible to depend cgroup v2?

May we implement both v1 and v2? Group v2 depends on the Linux kernel. On my machine, it mounts the cgroup v1 file system by default.

codecov · 2023-06-01T09:00:09Z

Codecov Report

Merging #415 (5d9bdba) into main (88ccb52) will increase coverage by 0.17%.
The diff coverage is 56.92%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #415      +/-   ##
==========================================
+ Coverage   37.70%   37.88%   +0.17%     
==========================================
  Files          59       60       +1     
  Lines        6996     7061      +65     
==========================================
+ Hits         2638     2675      +37     
- Misses       4047     4073      +26     
- Partials      311      313       +2

Impacted Files	Coverage Δ
config/config.go	`31.38% <0.00%> (-3.02%)`	⬇️
pkg/manager/daemon_adaptor.go	`0.00% <0.00%> (ø)`
pkg/manager/manager.go	`14.60% <0.00%> (-0.21%)`	⬇️
pkg/utils/parser/parser.go	`86.04% <86.04%> (ø)`

This PR introduce a cgroup interface for nydusd daemon Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

changweige · 2023-06-15T11:26:39Z

snapshot/snapshot.go

+		Name:   "nydusd",
+		Config: cgroupConfig,
+	})
+	if err != nil && (err != cgroup.ErrCgroupNotSupported || err != v2.ErrRootMemorySubtreeControllerNotEnableed) {


typo “Enableed” or this can be named to xxxDisabled

Good point. Fixed.

changweige · 2023-06-15T11:30:18Z

pkg/utils/sysinfo/sysinfo.go

+
+var (
+	sysinfo *syscall.Sysinfo_t
+)


Can you try sync.Once to initialize the global sysinfo?

It somewhat not threaded safe

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

changweige · 2023-06-15T11:39:47Z

pkg/cgroup/v2/v2.go

+}
+func (cg Cgroup) AddProc(pid int) error {
+	if cg.manager != nil {
+		err := cg.manager.AddProc(uint64(pid))


Need move the process out from controller if nydusd dies or exits normally?

The process will be automatically removed if nydusd dies. The cgroups library in containerd does not have this method either.

The parser package aims to parse configuration data to expected units Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

The cgroup manager aims to manager the lifetime of daemon's cgroup and add nydusd to this cgroup for memory resource limitation and reclaim. Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

refer to moby/moby#43093 Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

sctb512 mentioned this pull request Mar 16, 2023

change oom score adj for nydus daemon #404

Closed

imeoer reviewed Mar 16, 2023

View reviewed changes

changweige reviewed Mar 16, 2023

View reviewed changes

sctb512 changed the title ~~Limit nydus daemon memory via cgroup~~ [WIP] Limit nydus daemon memory via cgroup Mar 29, 2023

sctb512 closed this Jun 1, 2023

sctb512 force-pushed the limit-daemon-memory-via-cgroup branch from 2cb65c5 to d0d9f04 Compare June 1, 2023 08:57

sctb512 reopened this Jun 1, 2023

sctb512 force-pushed the limit-daemon-memory-via-cgroup branch 3 times, most recently from caab0fb to f20f46e Compare June 2, 2023 03:47

sctb512 changed the title ~~[WIP] Limit nydus daemon memory via cgroup~~ Limit nydus daemon memory via cgroup Jun 2, 2023

sctb512 force-pushed the limit-daemon-memory-via-cgroup branch 15 times, most recently from d3f42df to 66d32b1 Compare June 3, 2023 14:17

sctb512 force-pushed the limit-daemon-memory-via-cgroup branch 5 times, most recently from 5f00416 to 83d2797 Compare June 14, 2023 10:07

sctb512 added 2 commits June 14, 2023 18:08

pkg: introduce pkg/cgroup package

eda54d5

This PR introduce a cgroup interface for nydusd daemon Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

cgroup: add cgroup v1 implementation

9b373c0

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

sctb512 force-pushed the limit-daemon-memory-via-cgroup branch from 83d2797 to b81c6d7 Compare June 14, 2023 10:08

sctb512 requested a review from changweige June 14, 2023 11:17

changweige reviewed Jun 15, 2023

View reviewed changes

sctb512 added 2 commits June 15, 2023 19:35

cgroup: add cgroup v2 implementation

57509f4

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

config: add cgroup config entry for daemon

54fc5bc

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

changweige reviewed Jun 15, 2023

View reviewed changes

sctb512 force-pushed the limit-daemon-memory-via-cgroup branch 2 times, most recently from 1ec7551 to 438d963 Compare June 15, 2023 12:29

sctb512 added 11 commits June 15, 2023 20:37

pkg: introduce pkg/parser package

6030cab

The parser package aims to parse configuration data to expected units Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

config: parse cgroup configuration to expected units

7b055ff

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

config: add test for ParseCgroupConfig

8d70c00

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

daemon: add cgroup manager to daemon manager

d8d6dbe

The cgroup manager aims to manager the lifetime of daemon's cgroup and add nydusd to this cgroup for memory resource limitation and reclaim. Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

update go.mod to adapt pkg/cgroup package

18542aa

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

misc: add cgroup configuration for daemon

c6e22ec

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

workflow: add iontegration test for cgroup v2 environment

5964108

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

makefile(integration): enable cgroup capability for container

51615c4

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

integration: enable nesting for cgroup v2

a85f4fd

refer to moby/moby#43093 Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

config: add enable entry for nydusd cgroup

a5c6821

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

misc: add enable entry for nydusd cgroup

5d9bdba

Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>

sctb512 force-pushed the limit-daemon-memory-via-cgroup branch from 438d963 to 5d9bdba Compare June 15, 2023 12:37

changweige approved these changes Jun 16, 2023

View reviewed changes

changweige merged commit 0a9b4ad into containerd:main Jun 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit memory resource usage for nydus daemon via cgroup #415

Limit memory resource usage for nydus daemon via cgroup #415

sctb512 commented Mar 16, 2023 •

edited

Loading

imeoer Mar 16, 2023 •

edited

Loading

sctb512 Mar 16, 2023

changweige Jun 13, 2023

changweige left a comment

sctb512 commented Mar 16, 2023

changweige commented Mar 17, 2023

sctb512 commented Mar 17, 2023

codecov bot commented Jun 1, 2023 •

edited

Loading

changweige Jun 15, 2023

sctb512 Jun 15, 2023

changweige Jun 15, 2023

changweige Jun 15, 2023

sctb512 Jun 15, 2023

changweige Jun 15, 2023

sctb512 Jun 15, 2023

Limit memory resource usage for nydus daemon via cgroup #415

Limit memory resource usage for nydus daemon via cgroup #415

Conversation

sctb512 commented Mar 16, 2023 • edited Loading

imeoer Mar 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changweige left a comment

Choose a reason for hiding this comment

sctb512 commented Mar 16, 2023

changweige commented Mar 17, 2023

sctb512 commented Mar 17, 2023

codecov bot commented Jun 1, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sctb512 commented Mar 16, 2023 •

edited

Loading

imeoer Mar 16, 2023 •

edited

Loading

codecov bot commented Jun 1, 2023 •

edited

Loading