github-actions[bot] commented on code in PR #33550:
URL: https://github.com/apache/doris/pull/33550#discussion_r1560778524
##########
be/src/pipeline/exec/analytic_sink_operator.cpp:
##########
@@ -29,15 +29,21 @@ OPERATOR_CODE_GENERATOR(AnalyticSinkOperator,
StreamingOperator)
Status AnalyticSinkLocalState::init(RuntimeState* state, LocalSinkStateInfo&
info) {
RETURN_IF_ERROR(PipelineXSinkLocalState<AnalyticSharedState>::init(state,
info));
SCOPED_TIMER(exec_time_counter());
+ SCOPED_TIMER(_init_timer);
+ _blocks_memory_usage =
+ _profile->AddHighWaterMarkCounter("Blocks", TUnit::BYTES,
"MemoryUsage", 1);
+ _evaluation_timer = ADD_TIMER(profile(), "EvaluationTime");
+ return Status::OK();
+}
+
+Status AnalyticSinkLocalState::open(RuntimeState* state) {
Review Comment:
warning: method 'open' can be made static
[readability-convert-member-functions-to-static]
be/src/pipeline/exec/analytic_sink_operator.h:57:
```diff
- Status open(RuntimeState* state) override;
+ static Status open(RuntimeState* state) override;
```
##########
be/src/pipeline/exec/exchange_sink_operator.cpp:
##########
@@ -150,9 +151,24 @@
}
SCOPED_CONSUME_MEM_TRACKER(_mem_tracker.get());
+ // Make sure brpc stub is ready before execution.
+ for (int i = 0; i < channels.size(); ++i) {
+ RETURN_IF_ERROR(channels[i]->init_stub(state));
+ }
+ return Status::OK();
+}
+
+Status ExchangeSinkLocalState::open(RuntimeState* state) {
Review Comment:
warning: function 'open' exceeds recommended size/complexity thresholds
[readability-function-size]
```cpp
Status ExchangeSinkLocalState::open(RuntimeState* state) {
^
```
<details>
<summary>Additional context</summary>
**be/src/pipeline/exec/exchange_sink_operator.cpp:160:** 139 lines including
whitespace and comments (threshold 80)
```cpp
Status ExchangeSinkLocalState::open(RuntimeState* state) {
^
```
</details>
##########
be/src/pipeline/exec/distinct_streaming_aggregation_operator.cpp:
##########
@@ -68,6 +68,19 @@
DistinctStreamingAggLocalState::DistinctStreamingAggLocalState(RuntimeState* sta
Status DistinctStreamingAggLocalState::init(RuntimeState* state,
LocalStateInfo& info) {
RETURN_IF_ERROR(Base::init(state, info));
SCOPED_TIMER(Base::exec_time_counter());
+ SCOPED_TIMER(Base::_init_timer);
+ _build_timer = ADD_TIMER(Base::profile(), "BuildTime");
+ _exec_timer = ADD_TIMER(Base::profile(), "ExecTime");
+ _hash_table_compute_timer = ADD_TIMER(Base::profile(),
"HashTableComputeTime");
+ _hash_table_emplace_timer = ADD_TIMER(Base::profile(),
"HashTableEmplaceTime");
+ _hash_table_input_counter = ADD_COUNTER(Base::profile(),
"HashTableInputCount", TUnit::UNIT);
+
+ return Status::OK();
+}
+
+Status DistinctStreamingAggLocalState::open(RuntimeState* state) {
Review Comment:
warning: method 'open' can be made static
[readability-convert-member-functions-to-static]
be/src/pipeline/exec/distinct_streaming_aggregation_operator.h:45:
```diff
- Status open(RuntimeState* state) override;
+ static Status open(RuntimeState* state) override;
```
##########
be/src/pipeline/exec/sort_sink_operator.cpp:
##########
@@ -29,9 +29,18 @@ namespace doris::pipeline {
OPERATOR_CODE_GENERATOR(SortSinkOperator, StreamingOperator)
Status SortSinkLocalState::init(RuntimeState* state, LocalSinkStateInfo& info)
{
- RETURN_IF_ERROR(PipelineXSinkLocalState<SortSharedState>::init(state,
info));
+ RETURN_IF_ERROR(Base::init(state, info));
+ SCOPED_TIMER(exec_time_counter());
+ SCOPED_TIMER(_init_timer);
+ _sort_blocks_memory_usage =
+ ADD_CHILD_COUNTER_WITH_LEVEL(_profile, "SortBlocks", TUnit::BYTES,
"MemoryUsage", 1);
+ return Status::OK();
+}
+
+Status SortSinkLocalState::open(RuntimeState* state) {
Review Comment:
warning: method 'open' can be made static
[readability-convert-member-functions-to-static]
be/src/pipeline/exec/sort_sink_operator.h:59:
```diff
- Status open(RuntimeState* state) override;
+ static Status open(RuntimeState* state) override;
```
##########
be/src/pipeline/exec/repeat_operator.cpp:
##########
@@ -47,10 +47,10 @@ RepeatLocalState::RepeatLocalState(RuntimeState* state,
OperatorXBase* parent)
_child_block(vectorized::Block::create_unique()),
_repeat_id_idx(0) {}
-Status RepeatLocalState::init(RuntimeState* state, LocalStateInfo& info) {
- RETURN_IF_ERROR(Base::init(state, info));
+Status RepeatLocalState::open(RuntimeState* state) {
Review Comment:
warning: method 'open' can be made static
[readability-convert-member-functions-to-static]
be/src/pipeline/exec/repeat_operator.h:56:
```diff
- Status open(RuntimeState* state) override;
+ static Status open(RuntimeState* state) override;
```
##########
be/src/pipeline/exec/union_sink_operator.cpp:
##########
@@ -95,16 +95,24 @@ Status UnionSinkOperator::close(RuntimeState* state) {
Status UnionSinkLocalState::init(RuntimeState* state, LocalSinkStateInfo&
info) {
RETURN_IF_ERROR(Base::init(state, info));
+ SCOPED_TIMER(exec_time_counter());
+ SCOPED_TIMER(_init_timer);
+ auto& p = _parent->cast<Parent>();
+ _shared_state->data_queue.set_sink_dependency(_dependency,
p._cur_child_id);
+ return Status::OK();
+}
+
+Status UnionSinkLocalState::open(RuntimeState* state) {
Review Comment:
warning: method 'open' can be made static
[readability-convert-member-functions-to-static]
be/src/pipeline/exec/union_sink_operator.h:75:
```diff
- Status open(RuntimeState* state) override;
+ static Status open(RuntimeState* state) override;
```
##########
be/src/pipeline/exec/union_source_operator.cpp:
##########
@@ -124,6 +124,18 @@ Status UnionSourceLocalState::init(RuntimeState* state,
LocalStateInfo& info) {
_runtime_profile, "WaitForDependency[" + _dependency->name() +
"]Time", 1);
}
+ if (p.get_child_count() == 0) {
+ _dependency->set_ready();
+ }
+ return Status::OK();
+}
+
+Status UnionSourceLocalState::open(RuntimeState* state) {
Review Comment:
warning: method 'open' can be made static
[readability-convert-member-functions-to-static]
be/src/pipeline/exec/union_source_operator.h:80:
```diff
- Status open(RuntimeState* state) override;
+ static Status open(RuntimeState* state) override;
```
##########
be/src/pipeline/exec/exchange_sink_operator.cpp:
##########
@@ -150,9 +151,24 @@ Status ExchangeSinkLocalState::init(RuntimeState* state,
LocalSinkStateInfo& inf
}
SCOPED_CONSUME_MEM_TRACKER(_mem_tracker.get());
+ // Make sure brpc stub is ready before execution.
+ for (int i = 0; i < channels.size(); ++i) {
+ RETURN_IF_ERROR(channels[i]->init_stub(state));
+ }
+ return Status::OK();
+}
+
+Status ExchangeSinkLocalState::open(RuntimeState* state) {
Review Comment:
warning: function 'open' has cognitive complexity of 64 (threshold 50)
[readability-function-cognitive-complexity]
```cpp
Status ExchangeSinkLocalState::open(RuntimeState* state) {
^
```
<details>
<summary>Additional context</summary>
**be/src/pipeline/exec/exchange_sink_operator.cpp:162:** nesting level
increased to 1
```cpp
SCOPED_TIMER(_open_timer);
^
```
**be/src/util/runtime_profile.h:67:** expanded from macro 'SCOPED_TIMER'
```cpp
#define SCOPED_TIMER(c) ScopedTimer<MonotonicStopWatch>
MACRO_CONCAT(SCOPED_TIMER, __COUNTER__)(c)
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:163:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
RETURN_IF_ERROR(Base::open(state));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:163:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(Base::open(state));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:175:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if (_part_type == TPartitionType::UNPARTITIONED || _part_type ==
TPartitionType::RANDOM ||
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:194:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if ((_part_type == TPartitionType::UNPARTITIONED || channels.size() ==
1) &&
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:201:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
for (int i = 0; i < config::num_broadcast_buffer; ++i) {
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:207:** +1, nesting level
increased to 1
```cpp
} else if (local_size > 0) {
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:220:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if (_part_type == TPartitionType::HASH_PARTITIONED) {
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:224:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_partitioner->init(p._texprs));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:224:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_partitioner->init(p._texprs));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:225:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_partitioner->prepare(state, p._row_desc));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:225:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_partitioner->prepare(state, p._row_desc));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:228:** +1, nesting level
increased to 1
```cpp
} else if (_part_type ==
TPartitionType::BUCKET_SHFFULE_HASH_PARTITIONED) {
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:232:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_partitioner->init(p._texprs));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:232:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_partitioner->init(p._texprs));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:233:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_partitioner->prepare(state, p._row_desc));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:233:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_partitioner->prepare(state, p._row_desc));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:236:** +1, nesting level
increased to 1
```cpp
} else if (_part_type ==
TPartitionType::TABLET_SINK_SHUFFLE_PARTITIONED) {
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:242:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_schema->init(p._tablet_sink_schema));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:242:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_schema->init(p._tablet_sink_schema));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:244:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_vpartition->init());
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:244:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_vpartition->init());
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:267:** +1, nesting level
increased to 1
```cpp
} else if (_part_type == TPartitionType::TABLE_SINK_HASH_PARTITIONED) {
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:284:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_partitioner->init(p._texprs));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:284:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_partitioner->init(p._texprs));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:285:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_partitioner->prepare(state, p._row_desc));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:285:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_partitioner->prepare(state, p._row_desc));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:291:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if (_part_type == TPartitionType::HASH_PARTITIONED ||
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:294:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_partitioner->open(state));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:294:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_partitioner->open(state));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:295:** +1, nesting level
increased to 1
```cpp
} else if (_part_type ==
TPartitionType::TABLET_SINK_SHUFFLE_PARTITIONED) {
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:296:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_row_distribution.open(_tablet_sink_row_desc));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/exec/exchange_sink_operator.cpp:296:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_row_distribution.open(_tablet_sink_row_desc));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
</details>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
