You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "xinyiZzz (via GitHub)" <gi...@apache.org> on 2023/04/21 09:57:31 UTC
[GitHub] [doris] xinyiZzz opened a new pull request, #18908: [improvement](profile) Add LoadChannel runtime profile
xinyiZzz opened a new pull request, #18908:
URL: https://github.com/apache/doris/pull/18908
# Proposed changes
Issue Number: close #xxx
## Problem summary
like this:
TabletSink and LoadChannel in BE are M: N relationship,
Every once in a while LoadChannel will randomly return its own runtime profile to a TabletSink, so usually all LoadChannel runtime profiles are saved on each TabletSink, and the timeliness of the same LoadChannel profile saved on different TabletSinks is different, and each TabletSink will periodically send fe reports all the LoadChannel profiles saved by itself, and ensures to update the latest LoadChannel profile according to the timestamp.
## 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 pull request #18908: [improvement](profile) Insert into add LoadChannel runtime profile
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on PR #18908:
URL: https://github.com/apache/doris/pull/18908#issuecomment-1519093739
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 a diff in pull request #18908: [improvement](profile) Add LoadChannel runtime profile
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18908:
URL: https://github.com/apache/doris/pull/18908#discussion_r1173591681
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -221,17 +241,33 @@
}
int64_t TabletsChannel::mem_consumption() {
- int64_t mem_usage = 0;
+ int64_t write_mem_usage = 0;
+ int64_t flush_mem_usage = 0;
+ int64_t max_tablet_mem_usage = 0;
+ int64_t max_tablet_write_mem_usage = 0;
+ int64_t max_tablet_flush_mem_usage = 0;
{
std::lock_guard<SpinLock> l(_tablet_writers_lock);
_mem_consumptions.clear();
for (auto& it : _tablet_writers) {
- int64_t writer_mem = it.second->mem_consumption();
- mem_usage += writer_mem;
- _mem_consumptions.emplace(writer_mem, it.first);
+ int64_t write_mem = it.second->mem_consumption(MemType::WRITE);
+ write_mem_usage += write_mem;
+ int64_t flush_mem = it.second->mem_consumption(MemType::FLUSH);
+ flush_mem_usage += flush_mem;
+ if (write_mem > max_tablet_write_mem_usage) max_tablet_write_mem_usage = write_mem;
+ if (flush_mem > max_tablet_flush_mem_usage) max_tablet_flush_mem_usage = flush_mem;
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
if (flush_mem > max_tablet_flush_mem_usage) { max_tablet_flush_mem_usage = flush_mem;
}
```
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -221,17 +241,33 @@ void TabletsChannel::_close_wait(DeltaWriter* writer,
}
int64_t TabletsChannel::mem_consumption() {
- int64_t mem_usage = 0;
+ int64_t write_mem_usage = 0;
+ int64_t flush_mem_usage = 0;
+ int64_t max_tablet_mem_usage = 0;
+ int64_t max_tablet_write_mem_usage = 0;
+ int64_t max_tablet_flush_mem_usage = 0;
{
std::lock_guard<SpinLock> l(_tablet_writers_lock);
_mem_consumptions.clear();
for (auto& it : _tablet_writers) {
- int64_t writer_mem = it.second->mem_consumption();
- mem_usage += writer_mem;
- _mem_consumptions.emplace(writer_mem, it.first);
+ int64_t write_mem = it.second->mem_consumption(MemType::WRITE);
+ write_mem_usage += write_mem;
+ int64_t flush_mem = it.second->mem_consumption(MemType::FLUSH);
+ flush_mem_usage += flush_mem;
+ if (write_mem > max_tablet_write_mem_usage) max_tablet_write_mem_usage = write_mem;
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
if (write_mem > max_tablet_write_mem_usage) { max_tablet_write_mem_usage = write_mem;
}
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@ void RuntimeProfile::merge(RuntimeProfile* other) {
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
Review Comment:
warning: unknown type name 'PRuntimeProfileTree'; did you mean 'TRuntimeProfileTree'? [clang-diagnostic-error]
```suggestion
void RuntimeProfile::update(const TRuntimeProfileTree& proto_profile) {
```
**be/src/util/runtime_profile.h:47:** 'TRuntimeProfileTree' declared here
```cpp
class TRuntimeProfileTree;
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
Review Comment:
warning: type 'const std::vector<TRuntimeProfileNode>' does not provide a call operator [clang-diagnostic-error]
```cpp
proto_profile.nodes().end()};
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
+ DCHECK_LT(*idx, nodes.size());
+ const PRuntimeProfileNode& node = nodes[*idx];
+ {
+ std::lock_guard<std::mutex> l(_counter_map_lock);
+
+ for (const auto& counter : node.counters()) {
+ CounterMap::iterator j = _counter_map.find(counter.name());
+
+ if (j == _counter_map.end()) {
+ _counter_map[counter.name()] = _pool->add(
+ new Counter(static_cast<TUnit::type>(counter.type()), counter.value()));
+ } else {
+ if (j->second->type() != static_cast<TUnit::type>(counter.type())) {
+ LOG(ERROR) << "Cannot update protobuf counters with the same name (" << j->first
+ << ") but different types.";
+ } else {
+ j->second->set(counter.value());
+ }
+ }
+ }
+
+ for (auto& child_counter_src_itr : node.child_counters_map()) {
+ std::set<std::string>* child_counters = find_or_insert(
+ &_child_counter_map, child_counter_src_itr.first, std::set<std::string>());
+ child_counters->insert(child_counter_src_itr.second.child_counters_name().begin(),
+ child_counter_src_itr.second.child_counters_name().end());
+ }
+ }
+
+ {
+ std::lock_guard<std::mutex> l(_info_strings_lock);
+ const InfoStrings& info_strings = {node.info_strings().begin(), node.info_strings().end()};
+ for (const std::string& key : node.info_strings_display_order()) {
Review Comment:
warning: type 'const std::vector<std::string>' (aka 'const vector<basic_string<char>>') does not provide a call operator [clang-diagnostic-error]
```cpp
for (const std::string& key : node.info_strings_display_order()) {
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
+ DCHECK_LT(*idx, nodes.size());
+ const PRuntimeProfileNode& node = nodes[*idx];
+ {
+ std::lock_guard<std::mutex> l(_counter_map_lock);
+
+ for (const auto& counter : node.counters()) {
+ CounterMap::iterator j = _counter_map.find(counter.name());
+
+ if (j == _counter_map.end()) {
+ _counter_map[counter.name()] = _pool->add(
+ new Counter(static_cast<TUnit::type>(counter.type()), counter.value()));
+ } else {
+ if (j->second->type() != static_cast<TUnit::type>(counter.type())) {
+ LOG(ERROR) << "Cannot update protobuf counters with the same name (" << j->first
+ << ") but different types.";
+ } else {
+ j->second->set(counter.value());
+ }
+ }
+ }
+
+ for (auto& child_counter_src_itr : node.child_counters_map()) {
+ std::set<std::string>* child_counters = find_or_insert(
+ &_child_counter_map, child_counter_src_itr.first, std::set<std::string>());
+ child_counters->insert(child_counter_src_itr.second.child_counters_name().begin(),
+ child_counter_src_itr.second.child_counters_name().end());
+ }
+ }
+
+ {
+ std::lock_guard<std::mutex> l(_info_strings_lock);
+ const InfoStrings& info_strings = {node.info_strings().begin(), node.info_strings().end()};
Review Comment:
warning: type 'const std::map<std::string, std::string>' (aka 'const map<basic_string<char>, basic_string<char>>') does not provide a call operator [clang-diagnostic-error]
```cpp
const InfoStrings& info_strings = {node.info_strings().begin(), node.info_strings().end()};
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
+ DCHECK_LT(*idx, nodes.size());
+ const PRuntimeProfileNode& node = nodes[*idx];
+ {
+ std::lock_guard<std::mutex> l(_counter_map_lock);
+
+ for (const auto& counter : node.counters()) {
+ CounterMap::iterator j = _counter_map.find(counter.name());
+
+ if (j == _counter_map.end()) {
+ _counter_map[counter.name()] = _pool->add(
+ new Counter(static_cast<TUnit::type>(counter.type()), counter.value()));
+ } else {
+ if (j->second->type() != static_cast<TUnit::type>(counter.type())) {
+ LOG(ERROR) << "Cannot update protobuf counters with the same name (" << j->first
+ << ") but different types.";
+ } else {
+ j->second->set(counter.value());
+ }
+ }
+ }
+
+ for (auto& child_counter_src_itr : node.child_counters_map()) {
Review Comment:
warning: type 'const std::map<std::string, std::set<std::string>>' (aka 'const map<basic_string<char>, set<basic_string<char>>>') does not provide a call operator [clang-diagnostic-error]
```cpp
for (auto& child_counter_src_itr : node.child_counters_map()) {
^
```
##########
be/src/util/runtime_profile.h:
##########
@@ -272,6 +272,14 @@
// the key has already been registered.
void update(const TRuntimeProfileTree& thrift_profile);
+ // update a subtree of profiles from nodes, rooted at *idx.
+ // On return, *idx points to the node immediately following this subtree.
+ void update(const std::vector<TRuntimeProfileNode>& nodes, int* idx);
+
+ // Used to transfer profile between BE, same behavior as thrift update.
+ void update(const PRuntimeProfileTree& proto_profile);
Review Comment:
warning: unknown type name 'PRuntimeProfileTree'; did you mean 'TRuntimeProfileTree'? [clang-diagnostic-error]
```suggestion
void update(const TRuntimeProfileTree& proto_profile);
```
**be/src/util/runtime_profile.h:47:** 'TRuntimeProfileTree' declared here
```cpp
class TRuntimeProfileTree;
^
```
##########
be/src/util/runtime_profile.h:
##########
@@ -331,6 +339,10 @@
void to_thrift(TRuntimeProfileTree* tree);
void to_thrift(std::vector<TRuntimeProfileNode>* nodes);
+ // Used to transfer profile between BE, same behavior as to_thrift.
+ void to_proto(PRuntimeProfileTree* tree);
+ void to_proto(google::protobuf::RepeatedPtrField<PRuntimeProfileNode>* nodes) const;
Review Comment:
warning: unknown type name 'PRuntimeProfileNode'; did you mean 'TRuntimeProfileNode'? [clang-diagnostic-error]
```suggestion
void to_proto(google::protobuf::RepeatedPtrField<TRuntimeProfileNode>* nodes) const;
```
**be/src/util/runtime_profile.h:46:** 'TRuntimeProfileNode' declared here
```cpp
class TRuntimeProfileNode;
^
```
##########
be/src/util/runtime_profile.h:
##########
@@ -331,6 +339,10 @@
void to_thrift(TRuntimeProfileTree* tree);
void to_thrift(std::vector<TRuntimeProfileNode>* nodes);
+ // Used to transfer profile between BE, same behavior as to_thrift.
+ void to_proto(PRuntimeProfileTree* tree);
Review Comment:
warning: unknown type name 'PRuntimeProfileTree'; did you mean 'TRuntimeProfileTree'? [clang-diagnostic-error]
```suggestion
void to_proto(TRuntimeProfileTree* tree);
```
**be/src/util/runtime_profile.h:47:** 'TRuntimeProfileTree' declared here
```cpp
class TRuntimeProfileTree;
^
```
##########
be/src/runtime/fragment_mgr.cpp:
##########
@@ -402,7 +403,10 @@ void FragmentMgr::coordinator_callback(const ReportStatusRequest& req) {
params.__isset.profile = false;
} else {
req.profile->to_thrift(¶ms.profile);
+ if (req.load_channel_profile)
+ req.load_channel_profile->to_thrift(¶ms.loadChannelProfile);
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
if (req.load_channel_profile) {
req.load_channel_profile->to_thrift(¶ms.loadChannelProfile);
}
```
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -221,17 +241,33 @@
}
int64_t TabletsChannel::mem_consumption() {
- int64_t mem_usage = 0;
+ int64_t write_mem_usage = 0;
+ int64_t flush_mem_usage = 0;
+ int64_t max_tablet_mem_usage = 0;
+ int64_t max_tablet_write_mem_usage = 0;
+ int64_t max_tablet_flush_mem_usage = 0;
{
std::lock_guard<SpinLock> l(_tablet_writers_lock);
_mem_consumptions.clear();
for (auto& it : _tablet_writers) {
- int64_t writer_mem = it.second->mem_consumption();
- mem_usage += writer_mem;
- _mem_consumptions.emplace(writer_mem, it.first);
+ int64_t write_mem = it.second->mem_consumption(MemType::WRITE);
+ write_mem_usage += write_mem;
+ int64_t flush_mem = it.second->mem_consumption(MemType::FLUSH);
+ flush_mem_usage += flush_mem;
+ if (write_mem > max_tablet_write_mem_usage) max_tablet_write_mem_usage = write_mem;
+ if (flush_mem > max_tablet_flush_mem_usage) max_tablet_flush_mem_usage = flush_mem;
+ if (write_mem + flush_mem > max_tablet_mem_usage)
+ max_tablet_mem_usage = write_mem + flush_mem;
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
if (write_mem + flush_mem > max_tablet_mem_usage) {
max_tablet_mem_usage = write_mem + flush_mem;
}
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
Review Comment:
warning: unknown type name 'PRuntimeProfileNode'; did you mean 'TRuntimeProfileNode'? [clang-diagnostic-error]
```suggestion
void RuntimeProfile::update(const std::vector<TRuntimeProfileNode>& nodes, int* idx) {
```
**be/src/util/runtime_profile.h:46:** 'TRuntimeProfileNode' declared here
```cpp
class TRuntimeProfileNode;
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
+ DCHECK_LT(*idx, nodes.size());
+ const PRuntimeProfileNode& node = nodes[*idx];
+ {
+ std::lock_guard<std::mutex> l(_counter_map_lock);
+
+ for (const auto& counter : node.counters()) {
Review Comment:
warning: type 'const std::vector<TCounter>' does not provide a call operator [clang-diagnostic-error]
```cpp
for (const auto& counter : node.counters()) {
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
+ DCHECK_LT(*idx, nodes.size());
+ const PRuntimeProfileNode& node = nodes[*idx];
Review Comment:
warning: unknown type name 'PRuntimeProfileNode'; did you mean 'TRuntimeProfileNode'? [clang-diagnostic-error]
```suggestion
const TRuntimeProfileNode& node = nodes[*idx];
```
**be/src/util/runtime_profile.h:46:** 'TRuntimeProfileNode' declared here
```cpp
class TRuntimeProfileNode;
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
Review Comment:
warning: unknown type name 'PRuntimeProfileNode'; did you mean 'TRuntimeProfileNode'? [clang-diagnostic-error]
```suggestion
std::vector<TRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
```
**be/src/util/runtime_profile.h:46:** 'TRuntimeProfileNode' declared here
```cpp
class TRuntimeProfileNode;
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
Review Comment:
warning: type 'const std::vector<TRuntimeProfileNode>' does not provide a call operator [clang-diagnostic-error]
```cpp
std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
+ DCHECK_LT(*idx, nodes.size());
+ const PRuntimeProfileNode& node = nodes[*idx];
+ {
+ std::lock_guard<std::mutex> l(_counter_map_lock);
+
+ for (const auto& counter : node.counters()) {
+ CounterMap::iterator j = _counter_map.find(counter.name());
+
+ if (j == _counter_map.end()) {
+ _counter_map[counter.name()] = _pool->add(
+ new Counter(static_cast<TUnit::type>(counter.type()), counter.value()));
+ } else {
+ if (j->second->type() != static_cast<TUnit::type>(counter.type())) {
+ LOG(ERROR) << "Cannot update protobuf counters with the same name (" << j->first
+ << ") but different types.";
+ } else {
+ j->second->set(counter.value());
+ }
+ }
+ }
+
+ for (auto& child_counter_src_itr : node.child_counters_map()) {
+ std::set<std::string>* child_counters = find_or_insert(
+ &_child_counter_map, child_counter_src_itr.first, std::set<std::string>());
+ child_counters->insert(child_counter_src_itr.second.child_counters_name().begin(),
+ child_counter_src_itr.second.child_counters_name().end());
+ }
+ }
+
+ {
+ std::lock_guard<std::mutex> l(_info_strings_lock);
+ const InfoStrings& info_strings = {node.info_strings().begin(), node.info_strings().end()};
+ for (const std::string& key : node.info_strings_display_order()) {
+ // Look for existing info strings and update in place. If there
+ // are new strings, add them to the end of the display order.
+ // TODO: Is nodes.info_strings always a superset of
+ // _info_strings? If so, can just copy the display order.
+ InfoStrings::const_iterator it = info_strings.find(key);
+ DCHECK(it != info_strings.end());
+ InfoStrings::iterator existing = _info_strings.find(key);
+
+ if (existing == _info_strings.end()) {
+ _info_strings.insert(std::make_pair(key, it->second));
+ _info_strings_display_order.push_back(key);
+ } else {
+ _info_strings[key] = it->second;
+ }
+ }
+ }
+
+ ++*idx;
+ {
+ std::lock_guard<std::mutex> l(_children_lock);
+
+ // update children with matching names; create new ones if they don't match
+ for (int i = 0; i < node.num_children(); ++i) {
Review Comment:
warning: called object type 'int32_t' (aka 'int') is not a function or function pointer [clang-diagnostic-error]
```cpp
for (int i = 0; i < node.num_children(); ++i) {
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
+ DCHECK_LT(*idx, nodes.size());
+ const PRuntimeProfileNode& node = nodes[*idx];
+ {
+ std::lock_guard<std::mutex> l(_counter_map_lock);
+
+ for (const auto& counter : node.counters()) {
+ CounterMap::iterator j = _counter_map.find(counter.name());
+
+ if (j == _counter_map.end()) {
+ _counter_map[counter.name()] = _pool->add(
+ new Counter(static_cast<TUnit::type>(counter.type()), counter.value()));
+ } else {
+ if (j->second->type() != static_cast<TUnit::type>(counter.type())) {
+ LOG(ERROR) << "Cannot update protobuf counters with the same name (" << j->first
+ << ") but different types.";
+ } else {
+ j->second->set(counter.value());
+ }
+ }
+ }
+
+ for (auto& child_counter_src_itr : node.child_counters_map()) {
+ std::set<std::string>* child_counters = find_or_insert(
+ &_child_counter_map, child_counter_src_itr.first, std::set<std::string>());
+ child_counters->insert(child_counter_src_itr.second.child_counters_name().begin(),
+ child_counter_src_itr.second.child_counters_name().end());
+ }
+ }
+
+ {
+ std::lock_guard<std::mutex> l(_info_strings_lock);
+ const InfoStrings& info_strings = {node.info_strings().begin(), node.info_strings().end()};
Review Comment:
warning: type 'const std::map<std::string, std::string>' (aka 'const map<basic_string<char>, basic_string<char>>') does not provide a call operator [clang-diagnostic-error]
```cpp
const InfoStrings& info_strings = {node.info_strings().begin(), node.info_strings().end()};
^
```
##########
be/src/util/runtime_profile.h:
##########
@@ -272,6 +272,14 @@ class RuntimeProfile {
// the key has already been registered.
void update(const TRuntimeProfileTree& thrift_profile);
+ // update a subtree of profiles from nodes, rooted at *idx.
+ // On return, *idx points to the node immediately following this subtree.
+ void update(const std::vector<TRuntimeProfileNode>& nodes, int* idx);
+
+ // Used to transfer profile between BE, same behavior as thrift update.
+ void update(const PRuntimeProfileTree& proto_profile);
Review Comment:
warning: class member cannot be redeclared [clang-diagnostic-error]
```cpp
void update(const PRuntimeProfileTree& proto_profile);
^
```
**be/src/util/runtime_profile.h:272:** previous declaration is here
```cpp
void update(const TRuntimeProfileTree& thrift_profile);
^
```
##########
be/src/util/runtime_profile.h:
##########
@@ -272,6 +272,14 @@
// the key has already been registered.
void update(const TRuntimeProfileTree& thrift_profile);
+ // update a subtree of profiles from nodes, rooted at *idx.
+ // On return, *idx points to the node immediately following this subtree.
+ void update(const std::vector<TRuntimeProfileNode>& nodes, int* idx);
+
+ // Used to transfer profile between BE, same behavior as thrift update.
+ void update(const PRuntimeProfileTree& proto_profile);
+ void update(const std::vector<PRuntimeProfileNode>& nodes, int* idx);
Review Comment:
warning: class member cannot be redeclared [clang-diagnostic-error]
```cpp
void update(const std::vector<PRuntimeProfileNode>& nodes, int* idx);
^
```
**be/src/util/runtime_profile.h:276:** previous declaration is here
```cpp
void update(const std::vector<TRuntimeProfileNode>& nodes, int* idx);
^
```
##########
be/src/util/runtime_profile.cpp:
##########
@@ -125,6 +127,90 @@
}
}
+void RuntimeProfile::update(const PRuntimeProfileTree& proto_profile) {
+ int idx = 0;
+ std::vector<PRuntimeProfileNode> nodes = {proto_profile.nodes().begin(),
+ proto_profile.nodes().end()};
+ update(nodes, &idx);
+ DCHECK_EQ(idx, nodes.size());
+}
+
+void RuntimeProfile::update(const std::vector<PRuntimeProfileNode>& nodes, int* idx) {
+ DCHECK_LT(*idx, nodes.size());
+ const PRuntimeProfileNode& node = nodes[*idx];
+ {
+ std::lock_guard<std::mutex> l(_counter_map_lock);
+
+ for (const auto& counter : node.counters()) {
+ CounterMap::iterator j = _counter_map.find(counter.name());
+
+ if (j == _counter_map.end()) {
+ _counter_map[counter.name()] = _pool->add(
+ new Counter(static_cast<TUnit::type>(counter.type()), counter.value()));
+ } else {
+ if (j->second->type() != static_cast<TUnit::type>(counter.type())) {
+ LOG(ERROR) << "Cannot update protobuf counters with the same name (" << j->first
+ << ") but different types.";
+ } else {
+ j->second->set(counter.value());
+ }
+ }
+ }
+
+ for (auto& child_counter_src_itr : node.child_counters_map()) {
+ std::set<std::string>* child_counters = find_or_insert(
+ &_child_counter_map, child_counter_src_itr.first, std::set<std::string>());
+ child_counters->insert(child_counter_src_itr.second.child_counters_name().begin(),
+ child_counter_src_itr.second.child_counters_name().end());
+ }
+ }
+
+ {
+ std::lock_guard<std::mutex> l(_info_strings_lock);
+ const InfoStrings& info_strings = {node.info_strings().begin(), node.info_strings().end()};
+ for (const std::string& key : node.info_strings_display_order()) {
+ // Look for existing info strings and update in place. If there
+ // are new strings, add them to the end of the display order.
+ // TODO: Is nodes.info_strings always a superset of
+ // _info_strings? If so, can just copy the display order.
+ InfoStrings::const_iterator it = info_strings.find(key);
+ DCHECK(it != info_strings.end());
+ InfoStrings::iterator existing = _info_strings.find(key);
+
+ if (existing == _info_strings.end()) {
+ _info_strings.insert(std::make_pair(key, it->second));
+ _info_strings_display_order.push_back(key);
+ } else {
+ _info_strings[key] = it->second;
+ }
+ }
+ }
+
+ ++*idx;
+ {
+ std::lock_guard<std::mutex> l(_children_lock);
+
+ // update children with matching names; create new ones if they don't match
+ for (int i = 0; i < node.num_children(); ++i) {
+ const PRuntimeProfileNode& pchild = nodes[*idx];
Review Comment:
warning: unknown type name 'PRuntimeProfileNode'; did you mean 'TRuntimeProfileNode'? [clang-diagnostic-error]
```suggestion
const TRuntimeProfileNode& pchild = nodes[*idx];
```
**be/src/util/runtime_profile.h:46:** 'TRuntimeProfileNode' declared here
```cpp
class TRuntimeProfileNode;
^
```
##########
be/src/util/runtime_profile.h:
##########
@@ -272,6 +272,14 @@
// the key has already been registered.
void update(const TRuntimeProfileTree& thrift_profile);
+ // update a subtree of profiles from nodes, rooted at *idx.
+ // On return, *idx points to the node immediately following this subtree.
+ void update(const std::vector<TRuntimeProfileNode>& nodes, int* idx);
+
+ // Used to transfer profile between BE, same behavior as thrift update.
+ void update(const PRuntimeProfileTree& proto_profile);
+ void update(const std::vector<PRuntimeProfileNode>& nodes, int* idx);
Review Comment:
warning: unknown type name 'PRuntimeProfileNode'; did you mean 'TRuntimeProfileNode'? [clang-diagnostic-error]
```suggestion
void update(const std::vector<TRuntimeProfileNode>& nodes, int* idx);
```
**be/src/util/runtime_profile.h:46:** 'TRuntimeProfileNode' declared here
```cpp
class TRuntimeProfileNode;
^
```
--
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 #18908: [improvement](profile) Insert into add LoadChannel runtime profile
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #18908:
URL: https://github.com/apache/doris/pull/18908
--
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 pull request #18908: [improvement](profile) Insert into add LoadChannel runtime profile
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on PR #18908:
URL: https://github.com/apache/doris/pull/18908#issuecomment-1519073947
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 a diff in pull request #18908: [improvement](profile) Insert into add LoadChannel runtime profile
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18908:
URL: https://github.com/apache/doris/pull/18908#discussion_r1173602289
##########
be/src/util/runtime_profile.cpp:
##########
@@ -625,6 +712,62 @@ std::string RuntimeProfile::print_json_info(const std::string& profile_name, std
return s.GetString();
}
+void RuntimeProfile::to_proto(PRuntimeProfileTree* tree) {
Review Comment:
warning: method 'to_proto' can be made const [readability-make-member-function-const]
be/src/util/runtime_profile.h:343:
```diff
- void to_proto(PRuntimeProfileTree* tree);
+ void to_proto(PRuntimeProfileTree* tree) const;
```
```suggestion
void RuntimeProfile::to_proto(PRuntimeProfileTree* tree) const {
```
--
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 commented on a diff in pull request #18908: [improvement](profile) Insert into add LoadChannel runtime profile
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18908:
URL: https://github.com/apache/doris/pull/18908#discussion_r1174521569
##########
gensrc/proto/runtime_profile.proto:
##########
@@ -0,0 +1,78 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
Review Comment:
do not add such structure, just use profile's thrift structure and serialize it to bytes and send the binary data with protobuf message.
--
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 pull request #18908: [improvement](profile) Insert into add LoadChannel runtime profile
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on PR #18908:
URL: https://github.com/apache/doris/pull/18908#issuecomment-1517611179
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 #18908: [improvement](profile) Insert into add LoadChannel runtime profile
Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on code in PR #18908:
URL: https://github.com/apache/doris/pull/18908#discussion_r1174586327
##########
gensrc/proto/runtime_profile.proto:
##########
@@ -0,0 +1,78 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
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