-
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
[feature](profilev2) Preliminary support for profilev2. #24881
Conversation
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
@@ -416,7 +420,7 @@ RuntimeProfile::Counter* RuntimeProfile::add_counter(const std::string& name, TU | |||
|
|||
DCHECK(parent_counter_name == ROOT_COUNTER || | |||
_counter_map.find(parent_counter_name) != _counter_map.end()); | |||
Counter* counter = _pool->add(new Counter(type, 0)); | |||
Counter* counter = _pool->add(new Counter(type, 0, level)); |
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.
warning: variable 'counter' is not initialized [cppcoreguidelines-init-variables]
Counter* counter = _pool->add(new Counter(type, 0, level)); | |
Counter* counter = nullptr = _pool->add(new Counter(type, 0, level)); |
_metadata = md; | ||
} | ||
|
||
bool is_set_metadata() const { return _is_set_metadata; } |
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.
warning: function 'is_set_metadata' should be marked [[nodiscard]] [modernize-use-nodiscard]
bool is_set_metadata() const { return _is_set_metadata; } | |
[[nodiscard]] bool is_set_metadata() const { return _is_set_metadata; } |
TeamCity be ut coverage result: |
(From new machine)TeamCity pipeline, clickbench performance test result: |
gensrc/thrift/RuntimeProfile.thrift
Outdated
@@ -25,6 +25,7 @@ struct TCounter { | |||
1: required string name | |||
2: required Metrics.TUnit type | |||
3: required i64 value | |||
4: required i64 level |
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.
should always be optional, never use required.
be/src/util/runtime_profile.h
Outdated
Counter* add_counter(const std::string& name, TUnit::type type) { | ||
return add_counter(name, type, ""); | ||
} | ||
|
||
Counter* add_counter_with_levle(const std::string& name, TUnit::type type, int64_t level) { |
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.
typo error
run buildall |
TeamCity be ut coverage result: |
(From new machine)TeamCity pipeline, clickbench performance test result: |
@@ -1217,7 +1217,6 @@ public String getNodeExplainString(String prefix, TExplainLevel detailLevel) { | |||
if (isPointQuery()) { | |||
output.append(prefix).append("SHORT-CIRCUIT"); | |||
} | |||
|
|||
return output.toString(); |
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.
not modify useless file
@@ -534,13 +508,18 @@ public String toString() { | |||
} | |||
|
|||
public String getSimpleString() { |
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.
rename to getLevel1Profile
if (node.timestamp != -1 && node.timestamp < timestamp) { | ||
return; | ||
} | ||
if (node.isSetMetadata()) { | ||
this.nodeid = (int) node.getMetadata(); |
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 think we should not depend on it.
Add a specific field in profile object named plan_node_id.
metadata is too normal.
run buildall |
TeamCity be ut coverage result: |
(From new machine)TeamCity pipeline, clickbench performance test result: |
run buildall |
(From new machine)TeamCity pipeline, clickbench performance test result: |
TeamCity be ut coverage result: |
@@ -278,6 +278,9 @@ RuntimeProfile* RuntimeProfile::create_child(const std::string& name, bool inden | |||
std::lock_guard<std::mutex> l(_children_lock); | |||
DCHECK(_child_map.find(name) == _child_map.end()); | |||
RuntimeProfile* child = _pool->add(new RuntimeProfile(name)); | |||
if (this->is_set_metadata()) { |
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.
这样做可能是错的,因为如果meta data 里是node id,那么child 会有自己的node id
run buildall |
run buildall |
TeamCity be ut coverage result: |
(From new machine)TeamCity pipeline, clickbench performance test 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. |
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
You can set the level of counters on the backend using ADD_COUNTER_WITH_LEVEL/ADD_TIMER_WITH_LEVEL. The profile can then merge counters with level 1. set profile_level = 1; such as sql select count(*) from customer join item on c_customer_sk = i_item_sk profile Simple profile PLAN FRAGMENT 0 OUTPUT EXPRS: count(*) PARTITION: UNPARTITIONED VRESULT SINK MYSQL_PROTOCAL 7:VAGGREGATE (merge finalize) | output: count(partial_count(*))[apache#44] | group by: | cardinality=1 | TotalTime: avg 725.608us, max 725.608us, min 725.608us | RowsReturned: 1 | 6:VEXCHANGE offset: 0 TotalTime: avg 52.411us, max 52.411us, min 52.411us RowsReturned: 8 PLAN FRAGMENT 1 PARTITION: HASH_PARTITIONED: c_customer_sk STREAM DATA SINK EXCHANGE ID: 06 UNPARTITIONED TotalTime: avg 106.263us, max 118.38us, min 81.403us BlocksSent: 8 5:VAGGREGATE (update serialize) | output: partial_count(*)[apache#43] | group by: | cardinality=1 | TotalTime: avg 679.296us, max 739.395us, min 554.904us | BuildTime: avg 33.198us, max 48.387us, min 28.880us | ExecTime: avg 27.633us, max 40.278us, min 24.537us | RowsReturned: 8 | 4:VHASH JOIN | join op: INNER JOIN(PARTITIONED)[] | equal join conjunct: c_customer_sk = i_item_sk | runtime filters: RF000[bloom] <- i_item_sk(18000/16384/1048576) | cardinality=17,740 | vec output tuple id: 3 | vIntermediate tuple ids: 2 | hash output slot ids: 22 | RowsReturned: 18.0K (18000) | ProbeRows: 18.0K (18000) | ProbeTime: avg 862.308us, max 1.576ms, min 666.28us | BuildRows: 18.0K (18000) | BuildTime: avg 3.8ms, max 3.860ms, min 2.317ms | |----1:VEXCHANGE | offset: 0 | TotalTime: avg 48.822us, max 67.459us, min 30.380us | RowsReturned: 18.0K (18000) | 3:VEXCHANGE offset: 0 TotalTime: avg 33.162us, max 39.480us, min 28.854us RowsReturned: 18.0K (18000) PLAN FRAGMENT 2 PARTITION: HASH_PARTITIONED: c_customer_id STREAM DATA SINK EXCHANGE ID: 03 HASH_PARTITIONED: c_customer_sk TotalTime: avg 753.954us, max 1.210ms, min 499.470us BlocksSent: 64 2:VOlapScanNode TABLE: default_cluster:tpcds.customer(customer), PREAGGREGATION: ON runtime filters: RF000[bloom] -> c_customer_sk partitions=1/1, tablets=12/12, tabletList=1550745,1550747,1550749 ... cardinality=100000, avgRowSize=0.0, numNodes=1 pushAggOp=NONE TotalTime: avg 18.417us, max 41.319us, min 10.189us RowsReturned: 18.0K (18000) --------- Co-authored-by: yiguolei <[email protected]>
Proposed changes
You can set the level of counters on the backend using ADD_COUNTER_WITH_LEVEL/ADD_TIMER_WITH_LEVEL. The profile can then merge counters with level 1.
set profile_level = 1;
such as
sql
select count(*) from customer join item on c_customer_sk = i_item_sk
profile
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...