[core] Readme doc for physical mode #49457

dentiny · 2024-12-27T07:40:45Z

Discussed with @rynewang offline, it would be nice to have a short doc illustrating cgroup struction and lifecycle.

Signed-off-by: dentiny <[email protected]>

src/ray/common/cgroup/README.md

rynewang · 2024-12-27T17:32:41Z

src/ray/common/cgroup/README.md

@@ -0,0 +1,35 @@
+### Ray core cgroup documentation


Why heading 3 and heading 4, instead of 1 and 2?

I start from header 4 and going bigger (3 -> 2 -> 1). Do you have a strong preference on that?

rynewang · 2024-12-27T17:41:48Z

src/ray/common/cgroup/README.md

+
+- The feature is built upon cgroup, which only supports linux;
+- Only cgroup v2 is supported, meanwhile ray also requires application to have write permission and cgroup v2 be mounted in rw mode;
+- At the initial version, ray caps max resource usage via heuristric estimation (TODO: support user passed-in value).


Make this more clear on what "heuristric estimation" we use. Move this to ## Physical execution mode on how to use.

e.g.

If physical execution mode is enabled, Ray uses cgroup to restrict resource usage. Now we only support using `memory` as cgroup `memory.max` to cap a task process (and all its subprocesses recursively)'s max memory usage. For example, @ray.remote(memory=500 * 1024 * 1024) def some_function(x): pass obj = some_function.remote() This function is limited by 500MiB memory usage, and if it tries to use more, it OOMs and fails.

Also add lifetimes of a cgroup vs task attempts. Describe how you put a (pre existing idle) worker into a cgroup, and when you move it out and remove a cgroup.

src/ray/common/cgroup/README.md

rynewang · 2024-12-27T17:44:32Z

src/ray/common/cgroup/README.md

+        /                          \
+.../system_cgroup           .../application_cgroup
+                            /                   \
+                .../default_cgroup/  .../<task_id>_<attempt_id>_cgroup (*N)


[nit] task_default, and task_<task_id>_<attempt_id> ?

In my latest commit I just call it default; do we need to differentiate task and actor here? Since from users' perspective they're different concepts, and cgroup is actually exposed to users.

src/ray/common/cgroup/README.md

rynewang · 2024-12-27T17:55:10Z

src/ray/common/cgroup/README.md

+- Only cgroup v2 is supported, meanwhile ray also requires application to have write permission and cgroup v2 be mounted in rw mode;
+- At the initial version, ray caps max resource usage via heuristric estimation (TODO: support user passed-in value).
+
+#### Implementation details


[general] I am a bit worried about cgroup leaks on (1) subprocesses holding the cgroup can't be deleted on task exit, (2) raylet crash. Do you have some good ideas on this?

For (1) we certainly don't want these procs. We already use linux subreaper to kill leaked procs via RAY_kill_child_processes_on_worker_exit_with_raylet_subreaper and maybe we can have another flag like RAY_kill_child_processes_on_worker_exit_with_cgroup so on cgroup exit we SIGKILL all procs in cgroup. WDYT?

For (2) I can't think of a good idea. If you SIGKILL raylet, all cgroups are destined to leak. Maybe we can just accept it. But if so we need to document it both in here and in the eventual user facing document.

Also we have a Q (3): do we want to restrict processes created by raylet to don't have permission to move their own cgroups? Well, Ray does not have a lot of security features so we might be OK with this.

(1) subprocesses holding the cgroup can't be deleted on task exit

I see, integrating the cgroup cleanup is a good idea, but we might need a way to get cgroup folder name with unknown PID. Or a hacky way is to have another setup of cgroups which cannot be deleted after task / actor completion.

For (2) I can't think of a good idea. If you SIGKILL raylet, all cgroups are destined to leak. Maybe we can just accept it. But if so we need to document it both in here and in the eventual user facing document.

I cannot think of a good way either.

Also we have a Q (3): do we want to restrict processes created by raylet to don't have permission to move their own cgroups? Well, Ray does not have a lot of security features so we might be OK with this.

Don't think it's something we could handle at the moment; theoretically we are not container-ize user process, so it could access everything in the node / filesystem.

[general] I am a bit worried about cgroup leaks on (1) subprocesses holding the cgroup can't be deleted on task exit

At the initial version, I would like to implement a simple RAII-style cgroup setup and cleanup for raylet.

Signed-off-by: dentiny <[email protected]>

rynewang

Added some comments and suggestions. The goal is: another software engineer can implement it out with this readme file. hence, in tomorrow they can debug it as well.

rynewang · 2024-12-28T05:10:46Z

src/ray/common/cgroup/README.md

+
+obj = some_function.remote()
+```
+This function is limited by 500MiB memory usage, and if it tries to use more, it OOMs and fails.


Q: what happens if you set memory=1 byte, and try to move a proc to such a cgroup? is it " proc move failure" or proc get killed instantly after moving?

rynewang · 2024-12-28T05:12:07Z

src/ray/common/cgroup/README.md

+Ray core supports a physical execution mode, which allows users to cap resource consumption for their applications.
+
+A few benefits:
+- It prevents application from eating up unlimited resource to starve other applications running on the same node;


[nit] I think these 2 bullet points are not parallel or mutual exclusive... and it's not "benefits" but more of a neutrual "behavior". Also it's not clear what is "application" in this line. does it mean non-Ray or other-Raylet processes?

Also it's not clear what is "application" in this line. does it mean non-Ray or other-Raylet processes?

I renamed to it "user application".

and it's not "benefits" but more of a neutrual "behavior"

Well it's the benefit / motivation compared to "no cgroup", which is the current implementation.

I think these 2 bullet points are not parallel or mutual exclusive...

A second read, these two bullet points are expression the similar meaning, so I combine them into one.

rynewang · 2024-12-28T05:12:58Z

src/ray/common/cgroup/README.md

+obj = some_function.remote()
+```
+This function is limited by 500MiB memory usage, and if it tries to use more, it OOMs and fails.
+  + If a task / actor is not annotated with resource usage, ray caps max resource usage via heuristric estimation.


can we make it dead clear on the "heuristric estimation"? that is, do we have any limitations on the default leaf cgroup?

I might mis-understand, but I added an implementation detail on how max resource is set.

src/ray/common/cgroup/README.md

rynewang · 2024-12-28T05:16:58Z

src/ray/common/cgroup/README.md

+        /                       \
+.../internal           .../application
+                        /            \
+                .../default/  .../<task_id>_<attempt_id> (*N)


[nit] remote the / after default ?

Q: is it more clear to use a tree like shape?

/sys/fs/cgroup/ray_node_<node_id> ├── internal └── application ├── default └── <task_id>_<attempt_id> (*N)

also specify the attempt_id is raylet-wide, or per-task.

also specify the attempt_id is raylet-wide, or per-task.

I haven't decided yet, leave a TODO to fill in later.

src/ray/common/cgroup/README.md

rynewang · 2024-12-28T05:20:42Z

src/ray/common/cgroup/README.md

+
+- Raylet is responsible to create cgroup folder at startup, and cleanup the folder at its destruction
+- Each ray node having their own cgroup folder, which contains the node id to differentiate with other raylet(s)
+- `/sys/fs/cgroup/ray_node_<node_id>/application` is where ray sets overall max resource for all application processes


say the max resource = what. it should default to 0.8 total mem, or a user specified mem resource amount on node start.

I think we've already mentioned several lines above? Anyway add more wording as you wish.

rynewang · 2024-12-28T05:21:01Z

src/ray/common/cgroup/README.md

+- Raylet is responsible to create cgroup folder at startup, and cleanup the folder at its destruction
+- Each ray node having their own cgroup folder, which contains the node id to differentiate with other raylet(s)
+- `/sys/fs/cgroup/ray_node_<node_id>/application` is where ray sets overall max resource for all application processes
+- If a task / actor execute with their max resource specified, they will be placed in a dedicated cgroup, identified by the task id and attempt id


write about the attempt id here

Wondering what are you expecting here? Anyway I added some wording on attempt id.

Signed-off-by: dentiny <[email protected]>

rynewang

LGTM, with a question left

rynewang · 2024-12-28T20:48:16Z

src/ray/common/cgroup/README.md

+
+- Each ray node having their own cgroup folder, which contains the node id to differentiate with other raylet(s); in detail, raylet is responsible to create cgroup folder `/sys/fs/cgroup/ray_node_<node_id>`, `/sys/fs/cgroup/ray_node_<node_id>/internal` and `/sys/fs/cgroup/ray_node_<node_id>/application` at startup, and cleans up the folder upon process exit;
+- `/sys/fs/cgroup/ray_node_<node_id>/application` is where ray sets overall max resource for all application processes
+  + The max resource respects users' input on node start, or a heuristic value 80% of all logical resource will be taken


This reads "if user does not set, we cap to 0.8 * logical resources == 0.64 physical resources", while I think we really mean:

"if user does not set, we cap to 1 * logical resources == 0.8 physical resources" ?

Sorry I don't understand, I haven't mentioned any physical resource in the doc, why do you think so?

dentiny added the go add ONLY when ready to merge, run all tests label Dec 27, 2024

dentiny requested review from jjyao and rynewang December 27, 2024 07:40

dentiny assigned jjyao and rynewang Dec 27, 2024

dentiny force-pushed the hjiang/cgroup-readme branch from 1bf9563 to 34184d3 Compare December 27, 2024 07:42

readme for cgroup

32610f7

Signed-off-by: dentiny <[email protected]>

dentiny force-pushed the hjiang/cgroup-readme branch from 34184d3 to 32610f7 Compare December 27, 2024 09:40

dentiny mentioned this pull request Dec 27, 2024

[core] Setup cgroup v2 in C++ #49416

Open

rynewang reviewed Dec 27, 2024

View reviewed changes

dentiny added 5 commits December 27, 2024 22:45

prerequisite fallback

bdfa41a

Signed-off-by: dentiny <[email protected]>

cgroup lifecycle

cecd490

Signed-off-by: dentiny <[email protected]>

rename ray_node_id to ray_node_node_id

b85a652

Signed-off-by: dentiny <[email protected]>

rename cgroup folder

c0af176

Signed-off-by: dentiny <[email protected]>

cgroup folder lifecycle

5c4c5df

Signed-off-by: dentiny <[email protected]>

dentiny requested a review from rynewang December 28, 2024 00:08

TODO on idle process

df3ce52

Signed-off-by: dentiny <[email protected]>

rynewang reviewed Dec 28, 2024

View reviewed changes

dentiny added 2 commits December 28, 2024 06:28

clarify application and rewrite benefit

b97a14b

Signed-off-by: dentiny <[email protected]>

add doc for implementation for heuristic max

6e18292

Signed-off-by: dentiny <[email protected]>

dentiny force-pushed the hjiang/cgroup-readme branch from 7859cd1 to 6e18292 Compare December 28, 2024 06:41

dentiny added 7 commits December 28, 2024 06:44

slightly reword prerequisite

35f2aba

Signed-off-by: dentiny <[email protected]>

destruction -> process exit

22b8246

Signed-off-by: dentiny <[email protected]>

explain attempt id

871bd85

Signed-off-by: dentiny <[email protected]>

explain how to take max resource value

e2de6c3

Signed-off-by: dentiny <[email protected]>

nit format

35108dd

Signed-off-by: dentiny <[email protected]>

Add TODO on attempt id content

09cef17

Signed-off-by: dentiny <[email protected]>

combine two sentence on cgroup folder

75c619c

Signed-off-by: dentiny <[email protected]>

dentiny requested a review from rynewang December 28, 2024 07:04

rynewang approved these changes Dec 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Readme doc for physical mode #49457

[core] Readme doc for physical mode #49457

dentiny commented Dec 27, 2024

rynewang Dec 27, 2024

dentiny Dec 27, 2024

rynewang Dec 27, 2024

dentiny Dec 27, 2024

rynewang Dec 27, 2024

dentiny Dec 27, 2024

rynewang Dec 27, 2024

dentiny Dec 28, 2024

dentiny Dec 28, 2024

dentiny Dec 28, 2024

dentiny Dec 28, 2024

rynewang left a comment

rynewang Dec 28, 2024

dentiny Dec 28, 2024

rynewang Dec 28, 2024

dentiny Dec 28, 2024

dentiny Dec 28, 2024

dentiny Dec 28, 2024 •

edited

Loading

rynewang Dec 28, 2024

dentiny Dec 28, 2024

rynewang Dec 28, 2024

rynewang Dec 28, 2024

rynewang Dec 28, 2024

dentiny Dec 28, 2024

rynewang Dec 28, 2024

dentiny Dec 28, 2024

rynewang Dec 28, 2024

dentiny Dec 28, 2024 •

edited

Loading

rynewang left a comment

rynewang Dec 28, 2024

dentiny Dec 29, 2024

[core] Readme doc for physical mode #49457

Are you sure you want to change the base?

[core] Readme doc for physical mode #49457

Conversation

dentiny commented Dec 27, 2024

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

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

rynewang left a 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

dentiny Dec 28, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dentiny Dec 28, 2024 • edited Loading

Choose a reason for hiding this comment

rynewang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dentiny Dec 28, 2024 •

edited

Loading

dentiny Dec 28, 2024 •

edited

Loading