You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "luozenglin (via GitHub)" <gi...@apache.org> on 2023/05/11 03:26:34 UTC
[GitHub] [doris] luozenglin opened a new pull request, #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
luozenglin opened a new pull request, #19526:
URL: https://github.com/apache/doris/pull/19526
# Proposed changes
Issue Number: close #xxx
## Problem summary
Describe your changes.
## Checklist(Required)
* [ ] Does it affect the original behavior
* [ ] Has unit tests been added
* [ ] Has document been added or modified
* [ ] Does it need to update dependencies
* [ ] Is this PR support rollback (If NO, please explain WHY)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] xinyiZzz commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1193586133
##########
be/src/runtime/memory/mem_tracker_limiter.cpp:
##########
@@ -474,12 +473,46 @@ int64_t MemTrackerLimiter::free_top_overcommit_query(
return freed_mem;
}
-void MemTrackerLimiter::add_to_task_group(taskgroup::TaskGroupPtr task_group) {
- if (_task_group) {
- return;
+int64_t MemTrackerLimiter::tg_memory_limit_gc(
Review Comment:
Wouldn't it be better to put this in `mem_info.cpp`
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1543926843
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192954805
##########
be/src/runtime/task_group/task_group.h:
##########
@@ -35,14 +35,15 @@ class PipelineTask;
}
class TPipelineResourceGroup;
+class MemTrackerLimiter;
+struct TrackerLimiterGroup;
+using TrackerLimiterGroups = std::vector<TrackerLimiterGroup>;
Review Comment:
A query memtracker can be both held by TaskGroups memtacker list and Global's mem_tracker_limiter_pool, so I think here should be ```std::vector<shared_ptr<TrackerLimiterGroup>>```;
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1543279937
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1543930841
clang-tidy review says "All clean, LGTM! :+1:"
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192950326
##########
be/src/runtime/task_group/task_group_manager.cpp:
##########
@@ -35,20 +36,31 @@ TaskGroupManager* TaskGroupManager::instance() {
TaskGroupPtr TaskGroupManager::get_or_create_task_group(const TaskGroupInfo& task_group_info) {
{
std::shared_lock<std::shared_mutex> r_lock(_group_mutex);
- if (_task_groups.count(task_group_info._id)) {
- return _task_groups[task_group_info._id];
+ if (LIKELY(_task_groups.count(task_group_info.id))) {
+ auto task_group = _task_groups[task_group_info.id];
+ task_group->check_and_update(task_group_info);
+ return task_group;
}
}
- auto new_task_group =
- std::make_shared<TaskGroup>(task_group_info._id, task_group_info._name,
- task_group_info._cpu_share, task_group_info._version);
+ auto new_task_group = std::make_shared<TaskGroup>(task_group_info);
std::lock_guard<std::shared_mutex> w_lock(_group_mutex);
- if (_task_groups.count(task_group_info._id)) {
- return _task_groups[task_group_info._id];
+ if (_task_groups.count(task_group_info.id)) {
+ auto task_group = _task_groups[task_group_info.id];
+ task_group->check_and_update(task_group_info);
+ return task_group;
}
- _task_groups[task_group_info._id] = new_task_group;
+ _task_groups[task_group_info.id] = new_task_group;
return new_task_group;
}
+int64_t TaskGroupManager::memory_limit_gc() {
+ int64_t total_free_memory = 0;
+ std::shared_lock<std::shared_mutex> r_lock(_group_mutex);
Review Comment:
It seems that we can't create ResourceGroup when exec GC;
So how about use a copy-on-write.
Get a copy list of current RS under RLock, then GC current RS.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192152621
##########
be/src/common/daemon.cpp:
##########
@@ -230,9 +231,13 @@ void Daemon::memory_gc_thread() {
if (!MemInfo::initialized() || !ExecEnv::GetInstance()->initialized()) {
continue;
}
+ auto sys_mem_available = doris::MemInfo::sys_mem_available();
+
Review Comment:
done
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] xinyiZzz commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1191967326
##########
be/src/runtime/memory/mem_tracker.h:
##########
@@ -154,13 +154,13 @@ class MemTracker {
static void refresh_all_tracker_profile();
public:
- Snapshot make_snapshot() const;
+ virtual Snapshot make_snapshot() const;
Review Comment:
Why change it to `virtual`, where need to use `MemTrcker*` to call `MemTrckerLimiter*`
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192956627
##########
be/src/runtime/task_group/task_group.cpp:
##########
@@ -63,36 +72,105 @@ std::string TaskGroupEntity::debug_string() const {
cpu_share(), _queue.size(), _vruntime_ns);
}
-TaskGroup::TaskGroup(uint64_t id, std::string name, uint64_t cpu_share, int64_t version)
- : _id(id), _name(name), _cpu_share(cpu_share), _task_entity(this), _version(version) {}
+TaskGroup::TaskGroup(const TaskGroupInfo& tg_info)
+ : _id(tg_info.id),
+ _name(tg_info.name),
+ _cpu_share(tg_info.cpu_share),
+ _memory_limit(tg_info.memory_limit),
+ _version(tg_info.version),
+ _task_entity(this),
+ _mem_tracker_limiter_pool(MEM_TRACKER_GROUP_NUM) {}
std::string TaskGroup::debug_string() const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return fmt::format("TG[id = {}, name = {}, cpu_share = {}, version = {}]", _id, _name,
- cpu_share(), _version);
-}
-
-bool TaskGroup::check_version(int64_t version) const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return version > _version;
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ return fmt::format("TG[id = {}, name = {}, cpu_share = {}, memory_limit = {}, version = {}]",
+ _id, _name, cpu_share(), PrettyPrinter::print(_memory_limit, TUnit::BYTES),
+ _version);
}
void TaskGroup::check_and_update(const TaskGroupInfo& tg_info) {
- if (tg_info._id != _id) {
+ if (UNLIKELY(tg_info.id != _id)) {
return;
}
+ {
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ if (LIKELY(tg_info.version <= _version)) {
+ return;
+ }
+ }
- std::lock_guard<std::shared_mutex> wl {mutex};
- if (tg_info._version > _version) {
- _name = tg_info._name;
- _cpu_share = tg_info._cpu_share;
- _version = tg_info._version;
+ std::lock_guard<std::shared_mutex> wl {_mutex};
+ if (tg_info.version > _version) {
+ _name = tg_info.name;
+ _version = tg_info.version;
+ _memory_limit = tg_info.memory_limit;
+ if (_cpu_share != tg_info.cpu_share) {
+ ExecEnv::GetInstance()->pipeline_task_group_scheduler()->update_tg_cpu_share(
+ tg_info, shared_from_this());
+ }
}
}
+void TaskGroup::update_cpu_share_unlock(const TaskGroupInfo& tg_info) {
+ _cpu_share = tg_info.cpu_share;
+}
+
+std::list<MemTrackerLimiter*>::iterator TaskGroup::add_mem_tracker_limiter(
+ MemTrackerLimiter* mem_tracker_ptr, int64_t group_num) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ return _mem_tracker_limiter_pool[group_num].trackers.insert(
+ _mem_tracker_limiter_pool[group_num].trackers.end(), mem_tracker_ptr);
+}
+
+void TaskGroup::remove_mem_tracker_limiter(int64_t group_num,
+ const std::list<MemTrackerLimiter*>::iterator& iter) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ _mem_tracker_limiter_pool[group_num].trackers.erase(iter);
+}
+
+int64_t TaskGroup::memory_limit_gc() {
Review Comment:
From the perspective of code style, maybe it's better to move this method to mem_tracker_limiter.
RG GC logic can be more complex later, I think we can sperate it to another place, it could be current mem_tracker_limiter, or a ResourceGroupMemTrackerLimiter.
TaskGroup just need to hold task's info.
We can discuss it later.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192231810
##########
be/src/runtime/task_group/task_group.cpp:
##########
@@ -63,36 +72,105 @@ std::string TaskGroupEntity::debug_string() const {
cpu_share(), _queue.size(), _vruntime_ns);
}
-TaskGroup::TaskGroup(uint64_t id, std::string name, uint64_t cpu_share, int64_t version)
- : _id(id), _name(name), _cpu_share(cpu_share), _task_entity(this), _version(version) {}
+TaskGroup::TaskGroup(const TaskGroupInfo& tg_info)
+ : _id(tg_info.id),
+ _name(tg_info.name),
+ _cpu_share(tg_info.cpu_share),
+ _memory_limit(tg_info.memory_limit),
+ _version(tg_info.version),
+ _task_entity(this),
+ _mem_tracker_limiter_pool(MEM_TRACKER_GROUP_NUM) {}
std::string TaskGroup::debug_string() const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return fmt::format("TG[id = {}, name = {}, cpu_share = {}, version = {}]", _id, _name,
- cpu_share(), _version);
-}
-
-bool TaskGroup::check_version(int64_t version) const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return version > _version;
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ return fmt::format("TG[id = {}, name = {}, cpu_share = {}, memory_limit = {}, version = {}]",
+ _id, _name, cpu_share(), PrettyPrinter::print(_memory_limit, TUnit::BYTES),
+ _version);
}
void TaskGroup::check_and_update(const TaskGroupInfo& tg_info) {
- if (tg_info._id != _id) {
+ if (UNLIKELY(tg_info.id != _id)) {
return;
}
+ {
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ if (LIKELY(tg_info.version <= _version)) {
+ return;
+ }
+ }
- std::lock_guard<std::shared_mutex> wl {mutex};
- if (tg_info._version > _version) {
- _name = tg_info._name;
- _cpu_share = tg_info._cpu_share;
- _version = tg_info._version;
+ std::lock_guard<std::shared_mutex> wl {_mutex};
+ if (tg_info.version > _version) {
+ _name = tg_info.name;
+ _version = tg_info.version;
+ _memory_limit = tg_info.memory_limit;
+ if (_cpu_share != tg_info.cpu_share) {
+ ExecEnv::GetInstance()->pipeline_task_group_scheduler()->update_tg_cpu_share(
+ tg_info, shared_from_this());
+ }
}
}
+void TaskGroup::update_cpu_share_unlock(const TaskGroupInfo& tg_info) {
+ _cpu_share = tg_info.cpu_share;
+}
+
+std::list<MemTrackerLimiter*>::iterator TaskGroup::add_mem_tracker_limiter(
+ MemTrackerLimiter* mem_tracker_ptr, int64_t group_num) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ return _mem_tracker_limiter_pool[group_num].trackers.insert(
+ _mem_tracker_limiter_pool[group_num].trackers.end(), mem_tracker_ptr);
+}
+
+void TaskGroup::remove_mem_tracker_limiter(int64_t group_num,
+ const std::list<MemTrackerLimiter*>::iterator& iter) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ _mem_tracker_limiter_pool[group_num].trackers.erase(iter);
+}
+
+int64_t TaskGroup::memory_limit_gc() {
+ int64_t used_memory = 0;
+ for (auto& mem_tracker_group : _mem_tracker_limiter_pool) {
+ std::lock_guard<std::mutex> l(mem_tracker_group.group_lock);
+ for (const auto& tracker : mem_tracker_group.trackers) {
+ used_memory += tracker->consumption();
+ }
+ }
+
+ if (used_memory <= memory_limit()) {
Review Comment:
If Group A 's mem limit is 50%, Group B's mem limit is 50%
Group A actually uses 49%, Group B actually uses 49%
Then Group A and Group B's mem can not be released, even if BE occupies too much memory.
This is different from current logic;
In current logic, big query could be released.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1546824595
clang-tidy review says "All clean, LGTM! :+1:"
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1545951759
PR approved by at least one committer and no changes requested.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192231810
##########
be/src/runtime/task_group/task_group.cpp:
##########
@@ -63,36 +72,105 @@ std::string TaskGroupEntity::debug_string() const {
cpu_share(), _queue.size(), _vruntime_ns);
}
-TaskGroup::TaskGroup(uint64_t id, std::string name, uint64_t cpu_share, int64_t version)
- : _id(id), _name(name), _cpu_share(cpu_share), _task_entity(this), _version(version) {}
+TaskGroup::TaskGroup(const TaskGroupInfo& tg_info)
+ : _id(tg_info.id),
+ _name(tg_info.name),
+ _cpu_share(tg_info.cpu_share),
+ _memory_limit(tg_info.memory_limit),
+ _version(tg_info.version),
+ _task_entity(this),
+ _mem_tracker_limiter_pool(MEM_TRACKER_GROUP_NUM) {}
std::string TaskGroup::debug_string() const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return fmt::format("TG[id = {}, name = {}, cpu_share = {}, version = {}]", _id, _name,
- cpu_share(), _version);
-}
-
-bool TaskGroup::check_version(int64_t version) const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return version > _version;
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ return fmt::format("TG[id = {}, name = {}, cpu_share = {}, memory_limit = {}, version = {}]",
+ _id, _name, cpu_share(), PrettyPrinter::print(_memory_limit, TUnit::BYTES),
+ _version);
}
void TaskGroup::check_and_update(const TaskGroupInfo& tg_info) {
- if (tg_info._id != _id) {
+ if (UNLIKELY(tg_info.id != _id)) {
return;
}
+ {
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ if (LIKELY(tg_info.version <= _version)) {
+ return;
+ }
+ }
- std::lock_guard<std::shared_mutex> wl {mutex};
- if (tg_info._version > _version) {
- _name = tg_info._name;
- _cpu_share = tg_info._cpu_share;
- _version = tg_info._version;
+ std::lock_guard<std::shared_mutex> wl {_mutex};
+ if (tg_info.version > _version) {
+ _name = tg_info.name;
+ _version = tg_info.version;
+ _memory_limit = tg_info.memory_limit;
+ if (_cpu_share != tg_info.cpu_share) {
+ ExecEnv::GetInstance()->pipeline_task_group_scheduler()->update_tg_cpu_share(
+ tg_info, shared_from_this());
+ }
}
}
+void TaskGroup::update_cpu_share_unlock(const TaskGroupInfo& tg_info) {
+ _cpu_share = tg_info.cpu_share;
+}
+
+std::list<MemTrackerLimiter*>::iterator TaskGroup::add_mem_tracker_limiter(
+ MemTrackerLimiter* mem_tracker_ptr, int64_t group_num) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ return _mem_tracker_limiter_pool[group_num].trackers.insert(
+ _mem_tracker_limiter_pool[group_num].trackers.end(), mem_tracker_ptr);
+}
+
+void TaskGroup::remove_mem_tracker_limiter(int64_t group_num,
+ const std::list<MemTrackerLimiter*>::iterator& iter) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ _mem_tracker_limiter_pool[group_num].trackers.erase(iter);
+}
+
+int64_t TaskGroup::memory_limit_gc() {
+ int64_t used_memory = 0;
+ for (auto& mem_tracker_group : _mem_tracker_limiter_pool) {
+ std::lock_guard<std::mutex> l(mem_tracker_group.group_lock);
+ for (const auto& tracker : mem_tracker_group.trackers) {
+ used_memory += tracker->consumption();
+ }
+ }
+
+ if (used_memory <= memory_limit()) {
Review Comment:
If Group A 's mem is 50%, Group B's mem is 50%
Group A actually uses 49%, Group B actually uses 49%
Then Group A and Group B's mem can not be released, even if BE occupies too much memory.
This is different from current logic;
In current logic, big query could be released.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] xinyiZzz commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1191986336
##########
be/src/runtime/memory/mem_tracker_limiter.h:
##########
@@ -41,6 +42,21 @@
namespace doris {
+constexpr auto MEM_TRACKER_GROUP_NUM = 1000;
+
+namespace taskgroup {
+class TaskGroup;
+using TaskGroupPtr = std::shared_ptr<TaskGroup>;
Review Comment:
Why not put it in task_group.h
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192950326
##########
be/src/runtime/task_group/task_group_manager.cpp:
##########
@@ -35,20 +36,31 @@ TaskGroupManager* TaskGroupManager::instance() {
TaskGroupPtr TaskGroupManager::get_or_create_task_group(const TaskGroupInfo& task_group_info) {
{
std::shared_lock<std::shared_mutex> r_lock(_group_mutex);
- if (_task_groups.count(task_group_info._id)) {
- return _task_groups[task_group_info._id];
+ if (LIKELY(_task_groups.count(task_group_info.id))) {
+ auto task_group = _task_groups[task_group_info.id];
+ task_group->check_and_update(task_group_info);
+ return task_group;
}
}
- auto new_task_group =
- std::make_shared<TaskGroup>(task_group_info._id, task_group_info._name,
- task_group_info._cpu_share, task_group_info._version);
+ auto new_task_group = std::make_shared<TaskGroup>(task_group_info);
std::lock_guard<std::shared_mutex> w_lock(_group_mutex);
- if (_task_groups.count(task_group_info._id)) {
- return _task_groups[task_group_info._id];
+ if (_task_groups.count(task_group_info.id)) {
+ auto task_group = _task_groups[task_group_info.id];
+ task_group->check_and_update(task_group_info);
+ return task_group;
}
- _task_groups[task_group_info._id] = new_task_group;
+ _task_groups[task_group_info.id] = new_task_group;
return new_task_group;
}
+int64_t TaskGroupManager::memory_limit_gc() {
+ int64_t total_free_memory = 0;
+ std::shared_lock<std::shared_mutex> r_lock(_group_mutex);
Review Comment:
It seems that we can't create ResourceGroup when exec GC;
So how about using a copy-on-write.
Get a copy list of current RS under RLock, then GC current RS.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1191990367
##########
be/src/runtime/memory/mem_tracker_limiter.h:
##########
@@ -41,6 +42,21 @@
namespace doris {
+constexpr auto MEM_TRACKER_GROUP_NUM = 1000;
+
+namespace taskgroup {
+class TaskGroup;
+using TaskGroupPtr = std::shared_ptr<TaskGroup>;
Review Comment:
unable to pre-declare an alias
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192250619
##########
be/src/runtime/task_group/task_group.cpp:
##########
@@ -63,36 +72,105 @@ std::string TaskGroupEntity::debug_string() const {
cpu_share(), _queue.size(), _vruntime_ns);
}
-TaskGroup::TaskGroup(uint64_t id, std::string name, uint64_t cpu_share, int64_t version)
- : _id(id), _name(name), _cpu_share(cpu_share), _task_entity(this), _version(version) {}
+TaskGroup::TaskGroup(const TaskGroupInfo& tg_info)
+ : _id(tg_info.id),
+ _name(tg_info.name),
+ _cpu_share(tg_info.cpu_share),
+ _memory_limit(tg_info.memory_limit),
+ _version(tg_info.version),
+ _task_entity(this),
+ _mem_tracker_limiter_pool(MEM_TRACKER_GROUP_NUM) {}
std::string TaskGroup::debug_string() const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return fmt::format("TG[id = {}, name = {}, cpu_share = {}, version = {}]", _id, _name,
- cpu_share(), _version);
-}
-
-bool TaskGroup::check_version(int64_t version) const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return version > _version;
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ return fmt::format("TG[id = {}, name = {}, cpu_share = {}, memory_limit = {}, version = {}]",
+ _id, _name, cpu_share(), PrettyPrinter::print(_memory_limit, TUnit::BYTES),
+ _version);
}
void TaskGroup::check_and_update(const TaskGroupInfo& tg_info) {
- if (tg_info._id != _id) {
+ if (UNLIKELY(tg_info.id != _id)) {
return;
}
+ {
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ if (LIKELY(tg_info.version <= _version)) {
+ return;
+ }
+ }
- std::lock_guard<std::shared_mutex> wl {mutex};
- if (tg_info._version > _version) {
- _name = tg_info._name;
- _cpu_share = tg_info._cpu_share;
- _version = tg_info._version;
+ std::lock_guard<std::shared_mutex> wl {_mutex};
+ if (tg_info.version > _version) {
+ _name = tg_info.name;
+ _version = tg_info.version;
+ _memory_limit = tg_info.memory_limit;
+ if (_cpu_share != tg_info.cpu_share) {
+ ExecEnv::GetInstance()->pipeline_task_group_scheduler()->update_tg_cpu_share(
+ tg_info, shared_from_this());
+ }
}
}
+void TaskGroup::update_cpu_share_unlock(const TaskGroupInfo& tg_info) {
+ _cpu_share = tg_info.cpu_share;
+}
+
+std::list<MemTrackerLimiter*>::iterator TaskGroup::add_mem_tracker_limiter(
+ MemTrackerLimiter* mem_tracker_ptr, int64_t group_num) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ return _mem_tracker_limiter_pool[group_num].trackers.insert(
+ _mem_tracker_limiter_pool[group_num].trackers.end(), mem_tracker_ptr);
+}
+
+void TaskGroup::remove_mem_tracker_limiter(int64_t group_num,
+ const std::list<MemTrackerLimiter*>::iterator& iter) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ _mem_tracker_limiter_pool[group_num].trackers.erase(iter);
+}
+
+int64_t TaskGroup::memory_limit_gc() {
+ int64_t used_memory = 0;
+ for (auto& mem_tracker_group : _mem_tracker_limiter_pool) {
+ std::lock_guard<std::mutex> l(mem_tracker_group.group_lock);
+ for (const auto& tracker : mem_tracker_group.trackers) {
+ used_memory += tracker->consumption();
+ }
+ }
+
+ if (used_memory <= memory_limit()) {
Review Comment:
This pr does not affect the original gc logic. The situation you mentioned will be processed by doris::MemInfo::process_full_gc().
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1545951799
PR approved by anyone and no changes requested.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192956627
##########
be/src/runtime/task_group/task_group.cpp:
##########
@@ -63,36 +72,105 @@ std::string TaskGroupEntity::debug_string() const {
cpu_share(), _queue.size(), _vruntime_ns);
}
-TaskGroup::TaskGroup(uint64_t id, std::string name, uint64_t cpu_share, int64_t version)
- : _id(id), _name(name), _cpu_share(cpu_share), _task_entity(this), _version(version) {}
+TaskGroup::TaskGroup(const TaskGroupInfo& tg_info)
+ : _id(tg_info.id),
+ _name(tg_info.name),
+ _cpu_share(tg_info.cpu_share),
+ _memory_limit(tg_info.memory_limit),
+ _version(tg_info.version),
+ _task_entity(this),
+ _mem_tracker_limiter_pool(MEM_TRACKER_GROUP_NUM) {}
std::string TaskGroup::debug_string() const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return fmt::format("TG[id = {}, name = {}, cpu_share = {}, version = {}]", _id, _name,
- cpu_share(), _version);
-}
-
-bool TaskGroup::check_version(int64_t version) const {
- std::shared_lock<std::shared_mutex> rl {mutex};
- return version > _version;
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ return fmt::format("TG[id = {}, name = {}, cpu_share = {}, memory_limit = {}, version = {}]",
+ _id, _name, cpu_share(), PrettyPrinter::print(_memory_limit, TUnit::BYTES),
+ _version);
}
void TaskGroup::check_and_update(const TaskGroupInfo& tg_info) {
- if (tg_info._id != _id) {
+ if (UNLIKELY(tg_info.id != _id)) {
return;
}
+ {
+ std::shared_lock<std::shared_mutex> rl {_mutex};
+ if (LIKELY(tg_info.version <= _version)) {
+ return;
+ }
+ }
- std::lock_guard<std::shared_mutex> wl {mutex};
- if (tg_info._version > _version) {
- _name = tg_info._name;
- _cpu_share = tg_info._cpu_share;
- _version = tg_info._version;
+ std::lock_guard<std::shared_mutex> wl {_mutex};
+ if (tg_info.version > _version) {
+ _name = tg_info.name;
+ _version = tg_info.version;
+ _memory_limit = tg_info.memory_limit;
+ if (_cpu_share != tg_info.cpu_share) {
+ ExecEnv::GetInstance()->pipeline_task_group_scheduler()->update_tg_cpu_share(
+ tg_info, shared_from_this());
+ }
}
}
+void TaskGroup::update_cpu_share_unlock(const TaskGroupInfo& tg_info) {
+ _cpu_share = tg_info.cpu_share;
+}
+
+std::list<MemTrackerLimiter*>::iterator TaskGroup::add_mem_tracker_limiter(
+ MemTrackerLimiter* mem_tracker_ptr, int64_t group_num) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ return _mem_tracker_limiter_pool[group_num].trackers.insert(
+ _mem_tracker_limiter_pool[group_num].trackers.end(), mem_tracker_ptr);
+}
+
+void TaskGroup::remove_mem_tracker_limiter(int64_t group_num,
+ const std::list<MemTrackerLimiter*>::iterator& iter) {
+ std::lock_guard<std::mutex> l(_mem_tracker_limiter_pool[group_num].group_lock);
+ _mem_tracker_limiter_pool[group_num].trackers.erase(iter);
+}
+
+int64_t TaskGroup::memory_limit_gc() {
Review Comment:
From the perspective of code style, maybe it's better to move this method to mem_tracker_limiter.
RG GC logic can be more complex later, I think we can sperate it to another place, it could be current mem_tracker_limiter, or a ResourceGroupMemTrackerLimiter.
TaskGroup just need to hold TaskGroup's info.
We can discuss it later.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1545474230
clang-tidy review says "All clean, LGTM! :+1:"
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1547112647
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1547214710
PR approved by at least one committer and no changes requested.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192953642
##########
be/src/runtime/memory/mem_tracker_limiter.cpp:
##########
@@ -459,10 +468,18 @@ int64_t MemTrackerLimiter::free_top_overcommit_query(int64_t min_free_mem,
max_pq.pop();
}
if (!usage_strings.empty()) {
- LOG(INFO) << "Process GC Free Top Memory Overcommit " << TypeString[type] << ": "
+ LOG(INFO) << "Process GC Free Top Memory Overcommit " << type_string(type) << ": "
<< join(usage_strings, ",");
}
return freed_mem;
}
+void MemTrackerLimiter::add_to_task_group(taskgroup::TaskGroupPtr task_group) {
+ if (_task_group) {
Review Comment:
How about add DCHECK(_task_group == nullptr) here?
mem_tracker's task_group should be set only once.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192153198
##########
be/src/runtime/memory/mem_tracker_limiter.h:
##########
@@ -207,8 +251,13 @@ class MemTrackerLimiter final : public MemTracker {
// Iterator into mem_tracker_limiter_pool for this object. Stored to have O(1) remove.
std::list<MemTrackerLimiter*>::iterator _tracker_limiter_group_it;
+
+ taskgroup::TaskGroupPtr _task_group;
+ std::list<MemTrackerLimiter*>::iterator _tg_tracker_limiter_group_it;
};
+using MemTrackerLimiterPtr = std::shared_ptr<MemTrackerLimiter>;
Review Comment:
removed it
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192154888
##########
be/src/util/mem_info.cpp:
##########
@@ -122,8 +122,6 @@ bool MemInfo::process_minor_gc() {
MonotonicStopWatch watch;
watch.start();
int64_t freed_mem = 0;
- std::string vm_rss_str = PerfCounters::get_vm_rss_str();
- std::string mem_available_str = MemInfo::sys_mem_available_str();
Review Comment:
already modified, thanks
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1191994757
##########
be/src/runtime/memory/mem_tracker.h:
##########
@@ -154,13 +154,13 @@ class MemTracker {
static void refresh_all_tracker_profile();
public:
- Snapshot make_snapshot() const;
+ virtual Snapshot make_snapshot() const;
Review Comment:
Because this non-virtual function has been redefined in the derived class, this causes an `MemTrcker*` pointer to `MemTrckerLimiter*` and the `MemTrckerLimiter*` to call `make_snapshot` have inconsistent results.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1547113680
clang-tidy review says "All clean, LGTM! :+1:"
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1546824052
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] xinyiZzz commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1192124791
##########
be/src/util/mem_info.cpp:
##########
@@ -122,8 +122,6 @@ bool MemInfo::process_minor_gc() {
MonotonicStopWatch watch;
watch.start();
int64_t freed_mem = 0;
- std::string vm_rss_str = PerfCounters::get_vm_rss_str();
- std::string mem_available_str = MemInfo::sys_mem_available_str();
Review Comment:
Record the system memory status before GC for printing logs. Otherwise, "be mem < mem_limit" or `system available memory > water mark` may appear in the log during GC, causing confusion to users
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1545557504
run buildall
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] luozenglin merged pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "luozenglin (via GitHub)" <gi...@apache.org>.
luozenglin merged PR #19526:
URL: https://github.com/apache/doris/pull/19526
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] xinyiZzz commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1191985336
##########
be/src/runtime/memory/mem_tracker_limiter.h:
##########
@@ -207,8 +251,13 @@ class MemTrackerLimiter final : public MemTracker {
// Iterator into mem_tracker_limiter_pool for this object. Stored to have O(1) remove.
std::list<MemTrackerLimiter*>::iterator _tracker_limiter_group_it;
+
+ taskgroup::TaskGroupPtr _task_group;
+ std::list<MemTrackerLimiter*>::iterator _tg_tracker_limiter_group_it;
};
+using MemTrackerLimiterPtr = std::shared_ptr<MemTrackerLimiter>;
Review Comment:
doesn't seem to be used, can replace shared_ptr<>
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19526:
URL: https://github.com/apache/doris/pull/19526#issuecomment-1543278673
clang-tidy review says "All clean, LGTM! :+1:"
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] xinyiZzz commented on a diff in pull request #19526: [feature-wip](resource-group) Supports memory hard isolation of resource group
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on code in PR #19526:
URL: https://github.com/apache/doris/pull/19526#discussion_r1191861615
##########
be/src/common/daemon.cpp:
##########
@@ -230,9 +231,13 @@ void Daemon::memory_gc_thread() {
if (!MemInfo::initialized() || !ExecEnv::GetInstance()->initialized()) {
continue;
}
+ auto sys_mem_available = doris::MemInfo::sys_mem_available();
+
Review Comment:
Also need to save `doris::MemInfo::proc_mem_no_allocator_cache()`, minus the memory size released by Group gc
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org