-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[feat](metrics) Unify metrics of thread pool #43144
base: master
Are you sure you want to change the base?
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
run buildall |
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.
clang-tidy made some suggestions
TeamCity be ut coverage result: |
run buildall |
run buildall |
1 similar comment
run buildall |
TeamCity be ut coverage result: |
run buildall |
TeamCity be ut coverage result: |
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.
LGTM
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
7eb8642
to
5928144
Compare
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.
clang-tidy made some suggestions
5928144
to
2a36bd5
Compare
run buildall |
run buildall |
run buildall |
1 similar comment
run buildall |
06d35d8
to
f23c627
Compare
run buildall |
fc8a649
to
47474ba
Compare
run buildall |
be/src/util/threadpool.cpp
Outdated
#include "util/thread.h" | ||
|
||
namespace doris { | ||
|
||
// The name of these varialbs will be useds as metric name in prometheus. | ||
DEFINE_GAUGE_METRIC_PROTOTYPE_2ARG(thread_pool_running_tasks, MetricUnit::NOUNIT); |
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.
怎么区分不同thread pool的名字?
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.
在 label 里面区分
ada5aeb
to
2e538b3
Compare
run buildall |
2e538b3
to
614c0c9
Compare
run buildall |
TPC-H: Total hot run time: 40126 ms
|
TPC-DS: Total hot run time: 189275 ms
|
ClickBench: Total hot run time: 32.99 s
|
@@ -139,7 +139,6 @@ namespace doris { | |||
class PBackendService_Stub; | |||
class PFunctionService_Stub; | |||
|
|||
DEFINE_GAUGE_METRIC_PROTOTYPE_2ARG(scanner_thread_pool_queue_size, MetricUnit::NOUNIT); | |||
DEFINE_GAUGE_METRIC_PROTOTYPE_2ARG(send_batch_thread_pool_thread_num, MetricUnit::NOUNIT); |
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.
_send_batch_thread_pool 这个相关的监控,可以不考虑兼容性,直接迁移到通用的thread pool 监控里
#include "gutil/integral_types.h" | ||
|
||
namespace doris { | ||
|
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.
这个类要加单测
@@ -269,10 +286,35 @@ Status ThreadPool::init() { | |||
return status; | |||
} | |||
} | |||
|
|||
_metric_entity = DorisMetrics::instance()->metric_registry()->register_entity( |
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.
如果有两个workload group,里面都有scan thread pool, 此时register entity的时候,注册2次,但是pool 的名字是相同的,此时是什么行为?
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.
当一个workload group 删除的时候,调用deregister的时候,把这个entity 删除了,另外一个workload group 是什么行为?
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.
pool name不会相同,pool name 的后缀是 wg 的name
be/src/util/threadpool.cpp
Outdated
// Get the next token and task to execute. | ||
ThreadPoolToken* token = _queue.front(); | ||
_queue.pop_front(); | ||
DCHECK_EQ(ThreadPoolToken::State::RUNNING, token->state()); | ||
DCHECK(!token->_entries.empty()); | ||
Task task = std::move(token->_entries.front()); | ||
std::chrono::time_point<std::chrono::system_clock> current = |
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.
这个方法可能性能很差
return Status::OK(); | ||
} | ||
|
||
void ThreadPool::shutdown() { | ||
DorisMetrics::instance()->metric_registry()->deregister_entity(_metric_entity); |
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.
如果先deregister,假如此时还有task run,这个task run 结束之后,更新metric 会不会导致内存写脏?
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.
不会,metric的更新不是 thread pool做的,是当 Metrics 被外部系统请求的时候触发的更新,deregister 之后 map 里面就没有这个 metrics 了
614c0c9
to
d7e5f63
Compare
run buildall |
TPC-H: Total hot run time: 39916 ms
|
TPC-DS: Total hot run time: 198366 ms
|
ClickBench: Total hot run time: 32.67 s
|
What problem does this PR solve?
Add metrics for all thread pool, more specifically, for all ThreadPool objects.
All thread pool will have following metrics:
A new class
IntervalHistogramStat
is created for interval histogram calculation.Metrics is updated by
hook
method when they are needed by prometheus.Check List (For Committer)
Test
Behavior changed:
Does this need documentation?
Release note
None
Check List (For Reviewer who merge this PR)