You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/16 15:26:53 UTC
[GitHub] [doris] xinyiZzz opened a new pull request, #10924: [Enhancement] [Memory] Limit memory usage use process actual physical memory
xinyiZzz opened a new pull request, #10924:
URL: https://github.com/apache/doris/pull/10924
# Proposed changes
Issue Number: close #10923
## Problem Summary:
### solution
Limit process memory usage using the actual physical memory of the process in `/proc/self/status`.
This is independent of the consumption value of the mem tracker, which counts the virtual memory of the process malloc.
### motivation
Currently, the mem tracker uses the tcmalloc hook and the tcmalloc property `generic.total_physical_bytes` to limit process memory usage.
Both the tcmalloc hook and `generic.total_physical_bytes` record the total length of virtual memory of the process malloc, not the physical memory actually used by the process in the OS.
When there is a large amount of memory that is not used after application (this is abnormal code), the virtual memory of the process will be much larger than the physical memory actually used by the process, causing the mem tracker to limit the memory too strictly.
## Checklist(Required)
1. Does it affect the original behavior: (Yes)
2. Has unit tests been added: (No)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (No)
5. Are there any changes that cannot be rolled back: (No)
## 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 #10924: [Enhancement] [Memory] Limit memory usage use process actual physical memory
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #10924:
URL: https://github.com/apache/doris/pull/10924#discussion_r922926069
##########
be/src/runtime/exec_env_init.cpp:
##########
@@ -183,7 +183,6 @@ Status ExecEnv::_init_mem_tracker() {
<< ". Using physical memory instead";
global_memory_limit_bytes = MemInfo::physical_mem();
}
- MemTracker::get_process_tracker()->set_limit(global_memory_limit_bytes);
Review Comment:
yes, I modified the process tracker comment.
--
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 #10924: [Enhancement] [Memory] Limit memory usage use process actual physical memory
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #10924:
URL: https://github.com/apache/doris/pull/10924#discussion_r922924142
##########
be/src/http/action/compaction_action.h:
##########
@@ -75,8 +70,6 @@ class CompactionAction : public HttpHandler {
static std::mutex _compaction_running_mutex;
/// whether there is manual compaction running
static bool _is_compaction_running;
- /// memory tracker
- std::shared_ptr<MemTracker> _compaction_mem_tracker;
Review Comment:
Because `_compaction_mem_tracker` is not used,
Compaction memory is tracked by `StorageEngine::instance()->compaction_mem_tracker()`.
--
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] xiaokang commented on a diff in pull request #10924: [Enhancement] [Memory] Limit memory usage use process actual physical memory
Posted by GitBox <gi...@apache.org>.
xiaokang commented on code in PR #10924:
URL: https://github.com/apache/doris/pull/10924#discussion_r922917682
##########
be/src/util/perf_counters.h:
##########
@@ -95,6 +97,26 @@ class PerfCounters {
PerfCounters();
~PerfCounters();
+ // Refactor
+
+ struct ProcStatus {
+ int64_t vm_size = 0;
+ int64_t vm_peak = 0;
+ int64_t vm_rss = 0;
+ };
+
+ static int parser_int(const std::string& state_key);
Review Comment:
parse_xxx is better
##########
be/src/runtime/exec_env_init.cpp:
##########
@@ -183,7 +183,6 @@ Status ExecEnv::_init_mem_tracker() {
<< ". Using physical memory instead";
global_memory_limit_bytes = MemInfo::physical_mem();
}
- MemTracker::get_process_tracker()->set_limit(global_memory_limit_bytes);
Review Comment:
so we do NOT limit total memory by process tracker, and it's just used to track virtual memory of process, right?
##########
be/src/runtime/mem_tracker.h:
##########
@@ -112,7 +113,11 @@ class MemTracker {
static std::shared_ptr<MemTracker> get_temporary_mem_tracker(const std::string& label);
Status check_sys_mem_info(int64_t bytes) {
Review Comment:
is bytes still virtual memory?
##########
be/src/olap/rowset/segment_v2/segment.cpp:
##########
@@ -48,9 +48,10 @@ Status Segment::open(io::FileSystem* fs, const std::string& path, uint32_t segme
Segment::Segment(uint32_t segment_id, const TabletSchema* tablet_schema)
: _segment_id(segment_id), _tablet_schema(*tablet_schema) {
#ifndef BE_TEST
- _mem_tracker = StorageEngine::instance()->tablet_mem_tracker();
+ _mem_tracker = MemTracker::create_virtual_tracker(
+ -1, "Segment", StorageEngine::instance()->tablet_mem_tracker());
#else
- _mem_tracker = MemTracker::get_process_tracker();
Review Comment:
should process_tracker be used in other places?
##########
be/src/http/action/compaction_action.h:
##########
@@ -75,8 +70,6 @@ class CompactionAction : public HttpHandler {
static std::mutex _compaction_running_mutex;
/// whether there is manual compaction running
static bool _is_compaction_running;
- /// memory tracker
- std::shared_ptr<MemTracker> _compaction_mem_tracker;
Review Comment:
why remove _compaction_mem_tracker?
--
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 #10924: [Enhancement] [Memory] Limit memory usage use process actual physical memory
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #10924:
URL: https://github.com/apache/doris/pull/10924#discussion_r922923851
##########
be/src/util/perf_counters.h:
##########
@@ -95,6 +97,26 @@ class PerfCounters {
PerfCounters();
~PerfCounters();
+ // Refactor
+
+ struct ProcStatus {
+ int64_t vm_size = 0;
+ int64_t vm_peak = 0;
+ int64_t vm_rss = 0;
+ };
+
+ static int parser_int(const std::string& state_key);
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] yiguolei merged pull request #10924: [Enhancement] [Memory] Limit memory usage use process actual physical memory
Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #10924:
URL: https://github.com/apache/doris/pull/10924
--
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 #10924: [Enhancement] [Memory] Limit memory usage use process actual physical memory
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #10924:
URL: https://github.com/apache/doris/pull/10924#discussion_r922925932
##########
be/src/runtime/mem_tracker.h:
##########
@@ -112,7 +113,11 @@ class MemTracker {
static std::shared_ptr<MemTracker> get_temporary_mem_tracker(const std::string& label);
Status check_sys_mem_info(int64_t bytes) {
Review Comment:
yep, so it is still possible to predict OOM too early, when this virtual memory is not used.
--
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 #10924: [Enhancement] [Memory] Limit memory usage use process actual physical memory
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #10924:
URL: https://github.com/apache/doris/pull/10924#discussion_r922924890
##########
be/src/olap/rowset/segment_v2/segment.cpp:
##########
@@ -48,9 +48,10 @@ Status Segment::open(io::FileSystem* fs, const std::string& path, uint32_t segme
Segment::Segment(uint32_t segment_id, const TabletSchema* tablet_schema)
: _segment_id(segment_id), _tablet_schema(*tablet_schema) {
#ifndef BE_TEST
- _mem_tracker = StorageEngine::instance()->tablet_mem_tracker();
+ _mem_tracker = MemTracker::create_virtual_tracker(
+ -1, "Segment", StorageEngine::instance()->tablet_mem_tracker());
#else
- _mem_tracker = MemTracker::get_process_tracker();
Review Comment:
`process_tracker` should not be consumed by manual consume/release,
Manual consumption in Segment should create a new virtual tracker
(I'm refactoring mem tracker to avoid this)
--
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