github-actions[bot] commented on code in PR #39374:
URL: https://github.com/apache/doris/pull/39374#discussion_r1718025946
##########
be/src/agent/cgroup_cpu_ctl.h:
##########
@@ -48,18 +50,34 @@ class CgroupCpuCtl {
// for log
void get_cgroup_cpu_info(uint64_t* cpu_shares, int* cpu_hard_limit);
- virtual Status delete_unused_cgroup_path(std::set<uint64_t>& used_wg_ids)
= 0;
+ static Status delete_unused_cgroup_path(std::set<uint64_t>& used_wg_ids);
+
+ static std::unique_ptr<CgroupCpuCtl> create_cgroup_cpu_ctl(uint64_t wg_id);
+
+ static bool is_a_valid_cgroup_path(std::string cg_path);
+
+ static uint64_t cpu_soft_limit_default_value();
protected:
- Status write_cg_sys_file(std::string file_path, int value, std::string
msg, bool is_append);
+ Status write_cg_sys_file(std::string file_path, std::string value,
std::string msg,
+ bool is_append);
virtual Status modify_cg_cpu_hard_limit_no_lock(int cpu_hard_limit) = 0;
virtual Status modify_cg_cpu_soft_limit_no_lock(int cpu_shares) = 0;
- std::string _doris_cgroup_cpu_path;
- uint64_t _cpu_core_num = CpuInfo::num_cores();
- uint64_t _cpu_cfs_period_us = 100000;
+ Status add_thread_to_cgroup(std::string task_file);
+
+protected:
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
<details>
<summary>Additional context</summary>
**be/src/agent/cgroup_cpu_ctl.h:60:** previously declared here
```cpp
protected:
^
```
</details>
##########
be/src/agent/cgroup_cpu_ctl.h:
##########
@@ -48,18 +50,34 @@
// for log
void get_cgroup_cpu_info(uint64_t* cpu_shares, int* cpu_hard_limit);
- virtual Status delete_unused_cgroup_path(std::set<uint64_t>& used_wg_ids)
= 0;
+ static Status delete_unused_cgroup_path(std::set<uint64_t>& used_wg_ids);
+
+ static std::unique_ptr<CgroupCpuCtl> create_cgroup_cpu_ctl(uint64_t wg_id);
+
+ static bool is_a_valid_cgroup_path(std::string cg_path);
+
+ static uint64_t cpu_soft_limit_default_value();
protected:
- Status write_cg_sys_file(std::string file_path, int value, std::string
msg, bool is_append);
+ Status write_cg_sys_file(std::string file_path, std::string value,
std::string msg,
+ bool is_append);
virtual Status modify_cg_cpu_hard_limit_no_lock(int cpu_hard_limit) = 0;
virtual Status modify_cg_cpu_soft_limit_no_lock(int cpu_shares) = 0;
- std::string _doris_cgroup_cpu_path;
- uint64_t _cpu_core_num = CpuInfo::num_cores();
- uint64_t _cpu_cfs_period_us = 100000;
+ Status add_thread_to_cgroup(std::string task_file);
+
+protected:
+ inline static uint64_t _cpu_core_num;
+ const static uint64_t _cpu_cfs_period_us = 100000;
+ inline static std::string _doris_cgroup_cpu_path = "";
+ inline static std::string _doris_cgroup_cpu_query_path = "";
+ inline static bool _is_enable_cgroup_v1_in_env = false;
+ inline static bool _is_enable_cgroup_v2_in_env = false;
+ inline static bool _is_cgroup_query_path_valid = false;
+
+protected:
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
<details>
<summary>Additional context</summary>
**be/src/agent/cgroup_cpu_ctl.h:60:** previously declared here
```cpp
protected:
^
```
</details>
##########
be/src/agent/cgroup_cpu_ctl.h:
##########
@@ -96,20 +114,67 @@
class CgroupV1CpuCtl : public CgroupCpuCtl {
public:
CgroupV1CpuCtl(uint64_t tg_id) : CgroupCpuCtl(tg_id) {}
- CgroupV1CpuCtl() = default;
Status init() override;
Status modify_cg_cpu_hard_limit_no_lock(int cpu_hard_limit) override;
Status modify_cg_cpu_soft_limit_no_lock(int cpu_shares) override;
Status add_thread_to_cgroup() override;
- Status delete_unused_cgroup_path(std::set<uint64_t>& used_wg_ids) override;
-
private:
- std::string _cgroup_v1_cpu_query_path;
std::string _cgroup_v1_cpu_tg_path; // workload group path
std::string _cgroup_v1_cpu_tg_quota_file;
std::string _cgroup_v1_cpu_tg_shares_file;
std::string _cgroup_v1_cpu_tg_task_file;
};
+/*
+ NOTE: cgroup v2 directory structure
+ 1 root path:
+ /sys/fs/cgroup
+
+ 2 doris home path:
+ /sys/fs/cgroup/{doris_home}/
+
+ 3 doris home subtree_control file:
+ /sys/fs/cgroup/{doris_home}/cgroup.subtree_control
+
+ 4 query path:
+ /sys/fs/cgroup/{doris_home}/query/
+
+ 5 query path subtree_control file:
+ /sys/fs/cgroup/{doris_home}/query/cgroup.subtree_control
+
+ 6 workload group path:
+ /sys/fs/cgroup/{doris_home}/query/{workload_group_id}
+
+ 7 workload grou cpu.max file:
+ /sys/fs/cgroup/{doris_home}/query/{workload_group_id}/cpu.max
+
+ 8 workload grou cpu.weight file:
+ /sys/fs/cgroup/{doris_home}/query/{workload_group_id}/cpu.weight
+
+ 9 workload group cgroup type file:
+ /sys/fs/cgroup/{doris_home}/query/{workload_group_id}/cgroup.type
+
+*/
+class CgroupV2CpuCtl : public CgroupCpuCtl {
+public:
+ CgroupV2CpuCtl(uint64_t tg_id) : CgroupCpuCtl(tg_id) {}
+ Status init() override;
+ Status modify_cg_cpu_hard_limit_no_lock(int cpu_hard_limit) override;
+ Status modify_cg_cpu_soft_limit_no_lock(int cpu_shares) override;
+ Status add_thread_to_cgroup() override;
+
+private:
+ Status enable_cpu_controller(std::string file);
+
+private:
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
<details>
<summary>Additional context</summary>
**be/src/agent/cgroup_cpu_ctl.h:166:** previously declared here
```cpp
private:
^
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
