[core] Setup cgroup v2 in C++

[dentiny]

A rewrite of #48788

[rynewang]

is this default_cgroup_uuid the name for the default cgroup? the word uuid sounds strange to me, since this is a name but not a uuid.

[dentiny]

I choose the word uuid here since context contains a data field called uuid.

[rynewang]

So we need more designs on the API. For that, please describe how would you like to use this API, e.g. in WorkerPool or NodeManager. Write some pseudo code in worker / task lifetime to highlight usage of cgroup api.

[rynewang]

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

[dentiny]

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.

[rynewang]

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?

[dentiny]

Renamed as id.

[dentiny]

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

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

[rynewang]

before deleting the cgroup v2 we need to first move proc out of it, or the deletion would fail. for that you need to somehow record the prev cgroup if any, or we can blindly move all those procs to the default cgroup. this can come from a "global cgroup mgr" in raylet:

class CgroupV2Manager {

ctor(default_cgroup_name);

bool PutPidIntoDefaultCgroupRemovingAnyCgroupsIfAny(pid);
bool CreateCgroupV2ForPid(pid, cgroup_name);

};

[dentiny]

before deleting the cgroup v2 we need to first move proc out of it, or the deletion would fail.

Yes.. fixed.

[dentiny]

Discussed offline, I add a few items based on our offline discussion:

• I add a README for cgroup
• I add comments on local task manager on how I plan to integrate the cgroup RAII class with local task manager
• I add comments on how I plan to prepare cgroup basic setup in raylet

[rynewang]

@dentiny let's have a meeting to discuss about design of this PR.

[dentiny]

Offline discussion: need to document the precondition

• cgroup hierarchy under cgroup folder
• already exists default cgroup and permission

[dentiny]

Possible map to place cgroup:

ray/src/ray/core_worker/transport/normal_task_submitter.h

Lines 366 to 367 in c79de07

	// Keeps track of where currently executing tasks are being run.
	absl::flat_hash_map<TaskID, rpc::Address> executing_tasks_ ABSL_GUARDED_BY(mu_);

[dentiny]

Possible place to put cgroup setup class:

ray/src/ray/raylet/local_task_manager.cc

Lines 728 to 737 in c79de07

	void LocalTaskManager::RemoveFromRunningTasksIfExists(const RayTask &task) {
	auto sched_cls = task.GetTaskSpecification().GetSchedulingClass();
	auto it = info_by_sched_cls_.find(sched_cls);
	if (it != info_by_sched_cls_.end()) {
	it->second.running_tasks.erase(task.GetTaskSpecification().TaskId());
	if (it->second.running_tasks.size() == 0) {
	info_by_sched_cls_.erase(it);
	}
	}
	}

[dentiny]

Discussed offline with @rynewang , we hope two other items are included for easier review and roadmap:

• A readme to document prerequisite and directory structure: [core] Readme doc for physical mode #49457
• A brief no-op modification on cgroup util caller to illustrate how raylet creates/destructs cgroup

[rynewang]

Wait a minute... The running_tasks is inserted with task ID, before we call PopWorker. This means if you change running_tasks to a map { task ID -> cgroup raii }, the value must be nullptr at first, and is only set until PopWorker callback is called. I'm OK with this, but, @jjyao do you feel putting the cgroup raii (to remove proc from cgroup and delete cgroup on dtor) in LocalTaskManager's running_tasks is the best place? Is there a even more precise data structure somewhere in raylet to model a running task attempt?

[dentiny]

the value must be nullptr at first, and is only set until PopWorker callback is called

Yeah that's my plan

[dentiny]

A nullptr is needed anyway I think, for example, we need to handle cases where cgroup prerequisite is not met.

[dentiny]

@jjyao Do you have any thoughts?