-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Memory] Refactor MemTracker and new memory statistics framework based on TCMalloc Hook #7198
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.
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
Conversation
621fba5 to
92483d2
Compare
bee0e2d to
d99cf05
Compare
e96a3e8 to
c9b0280
Compare
|
Why does this parameter 'untracked_mem_limit_mbytes' affect performance? |
| fmt::format(detail, BackendOptions::get_localhost(), print_id(_fragment_instance_id), | ||
| std::to_string(_query_mem_tracker.lock()->consumption()), | ||
| std::to_string(_query_mem_tracker.lock()->limit())); | ||
| ExecEnv::GetInstance()->fragment_mgr()->cancel( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here loses the thread context and cannot cancel immediately.
The operator will continue to run until the flag is checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will pay attention to the problem that PlanFragmentExecutor::cancel cannot cancel instance in time.
After that, can consider printing the thread context recorded in ThreadContext, but it may be useless, because ThreadMemTracker will accumulate a batch of memory requests to call consume, and the memory request mem_limit_exceeded may be small.
It may be more effective to print the status of the query and all instance trackers.
Because compared to the previous only consume/release MemTracker for large memory, the consume/release frequency in TCMalloc Hook will be much higher. |
089c5d1 to
fc8210d
Compare
bd057fe to
023f6d6
Compare
| CONF_mInt32(remote_storage_read_buffer_mb, "16"); | ||
|
|
||
| // Whether to initialize TCmalloc new/delete Hook, MemTracker is currently counted in Hook. | ||
| CONF_mBool(use_tc_hook, "true"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
track_new_delete maybe better, is this can be mutable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, thats sounds good
6358705 to
f4b9675
Compare
f4b9675 to
37ce93a
Compare
37ce93a to
090665b
Compare
Proposed changes
(Unlike before, The query memory limit was previously used as a fragment instance level limit. Therefore, some previously successful large queries may fail, and the query memory limit configuration needs to be increased.)
[ impact of testing on performance ]
The current default untracked_mem_limit_mbytes = 1M, the test found no impact on performance
Config
untracked_mem_limit_mbytesimpact on performance:master, jmeter thread=50, Avg1: 23120, Avg2: 22505, Avg3: 23459
untracked_mem_limit_mbytes=4M, jmeter thread=50, Avg1: 23024, Avg2: 23391 22681
untracked_mem_limit_mbytes=2M, jmeter thread=50, Avg1: 23426, Avg2: 22674 23658
untracked_mem_limit_mbytes=64K, jmeter thread=50, Avg1: 24121, Avg2: 24042 23762 22939 23294
untracked_mem_limit_mbytes=4K, jmeter thread=50, Avg1: 24942, Avg2: 24983 23305 22611 23506
Types of changes
What types of changes does your code introduce to Doris?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...