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.

Sign up for GitHub

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

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

Merged
merged 23 commits into from
Jan 9, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a9e8dc6
setup cgroup v2 in C++
dentiny Dec 23, 2024
feb0282
strinv view for cgroup content
dentiny Dec 24, 2024
ce77002
rename cgroup -> cgroup v2
dentiny Dec 24, 2024
995dc0f
add comment for uuid
dentiny Dec 24, 2024
f54f2eb
minor update in ifdef macro
dentiny Dec 24, 2024
f3a4a9a
raii for cgroup v2
dentiny Dec 24, 2024
341317f
ifdef
dentiny Dec 26, 2024
50874cd
rename as id
dentiny Dec 26, 2024
3027ed8
comment on cgroup class
dentiny Dec 26, 2024
12f0cfe
fix comment
dentiny Dec 26, 2024
c5e3fd7
place process to default cgroup
dentiny Dec 26, 2024
b679ba7
comment to add process to application cgroup
dentiny Dec 27, 2024
7689889
add precondition requirement and fix cleanup logic
dentiny Dec 27, 2024
792fa54
leave TODO for string view
dentiny Dec 27, 2024
128bfdf
remove unused function
dentiny Dec 27, 2024
d3fdd2a
remove useless if branch
dentiny Dec 27, 2024
2dcd1e7
local manager change for cgroup
dentiny Dec 27, 2024
56aeea9
special handle EEXISTS
dentiny Jan 6, 2025
899b27b
Merge branch 'master' into hjiang/cgroup-utils-cpp
dentiny Jan 7, 2025
e15f30d
Merge branch 'master' into hjiang/cgroup-utils-cpp
dentiny Jan 8, 2025
8c0af7f
Merge branch 'master' into hjiang/cgroup-utils-cpp
dentiny Jan 8, 2025
2406936
Merge branch 'master' into hjiang/cgroup-utils-cpp
dentiny Jan 9, 2025
30fc386
Merge branch 'master' into hjiang/cgroup-utils-cpp
dentiny Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
strinv view for cgroup content 
Signed-off-by: dentiny <dentinyhao@gmail.com>
  • Loading branch information
@dentiny
dentiny committed Dec 24, 2024
commit feb0282f952fe9b504eb0224a1273f75cbe12aac
2 changes: 1 addition & 1 deletion src/ray/common/cgroup/cgroup_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace ray {

// Context used to setup cgroupv2 for a task / actor.
struct PhysicalModeExecutionContext {
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: do we need this separate config class from CgroupV2Setup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These data fields are necessary to construct and destruct cgroup;
As of now the struct doesn't seem that necessary since it only contains 4 fields and we could directly pass them into the factory function, but we could have much more fields (i.e. cpu-related, resource min / high), better to have a struct.

// Directory for cgroup, which is appled to application process.
// Directory for cgroup, which is applied to application process.
std::string_view cgroup_directory;
// UUID to indicate the current task / actor.
std::string uuid;
dentiny marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion src/ray/common/cgroup/cgroup_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace {
constexpr int kCgroupFilePerm = 0600;
dentiny marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a uuid, and we should not use a name like this visible in linux. We had the idea of getting a default name from cluster ID right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed as id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We had the idea of getting a default name from cluster ID right?

I'm not sure how cluster id is related here?

// Open a cgroup path and append write [content] into the file.
void OpenCgroupFileAndAppend(std::string_view path, std::string content) {
void OpenCgroupFileAndAppend(std::string_view path, std::string_view content) {
std::ofstream out_file{path.data(), std::ios::out | std::ios::app};
out_file << content;
}
Expand Down
Loading