[fix](counters) fix MemoryUsage and PeakMemoryUsage counters of some operators #41602

jacktengg · 2024-10-09T08:04:50Z

Proposed changes

Issue Number: close #xxx

doris-robot · 2024-10-09T08:04:56Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

jacktengg · 2024-10-09T08:05:22Z

run buildall

github-actions

clang-tidy made some suggestions

github-actions · 2024-10-09T08:12:00Z

be/src/pipeline/exec/operator.cpp

@@ -425,8 +424,7 @@ PipelineXSinkLocalStateBase::PipelineXSinkLocalStateBase(DataSinkOperatorXBase*
 }

 PipelineXLocalStateBase::PipelineXLocalStateBase(RuntimeState* state, OperatorXBase* parent)
-        : _num_rows_returned(0),
-          _rows_returned_counter(nullptr),
+        : _rows_returned_counter(nullptr),


warning: member initializer for '_rows_returned_counter' is redundant [modernize-use-default-member-init]

Suggested change

: _rows_returned_counter(nullptr),

: ,

jacktengg · 2024-10-09T08:58:05Z

run buildall

doris-robot · 2024-10-09T09:48:11Z

TeamCity be ut coverage result:
Function Coverage: 37.27% (9631/25839)
Line Coverage: 28.67% (79885/278658)
Region Coverage: 28.10% (41285/146945)
Branch Coverage: 24.72% (21034/85098)
Coverage Report: http://coverage.selectdb-in.cc/coverage/3fe3a78a697e6c02916f02d62d419cc25696226f_3fe3a78a697e6c02916f02d62d419cc25696226f/report/index.html

yiguolei · 2024-10-09T10:02:13Z

be/src/pipeline/exec/aggregation_sink_operator.cpp

                                   data.get_buffer_size_in_bytes() -
-                                   Base::_shared_state->mem_usage_record.used_in_state);
+                                   Base::_shared_state->mem_usage_record.used_in_state;
+                           Base::_mem_tracker->consume(arena_memory_usage);


base 里的这个memtracker 还有用么？

有了MemoryUsage和MemoryUsagePeak这两个counter，这个_mem_tracker感觉可以删掉了。

yiguolei · 2024-10-09T10:32:50Z

be/src/pipeline/exec/hashjoin_build_sink.cpp

+            local_state._memory_used_counter->set(local_state._mem_tracker->consumption());
+            local_state._peak_memory_usage_counter->set(
+                    local_state._mem_tracker->peak_consumption());
+            COUNTER_SET(local_state._build_blocks_memory_usage, (int64_t)new_block_mem_usage);


至少跟580 行一致

be/src/pipeline/exec/exchange_sink_operator.cpp

yiguolei · 2024-10-09T11:01:22Z

be/src/pipeline/exec/exchange_sink_buffer.cpp

@@ -386,6 +389,8 @@ Status ExchangeSinkBuffer::_send_rpc(InstanceLoId id) {
            }
        }
        if (request.block_holder->get_block()) {
+            _parent->memory_used_counter()->update(
+                    -request.block_holder->get_block()->ByteSizeLong());


broadcast 的时候，是shared ptr，一个block 在多个channel 共享的，你这么统计，结果很大。
先别统计broadcast的了。

yiguolei · 2024-10-09T11:34:01Z

be/src/pipeline/exec/exchange_sink_buffer.cpp

@@ -166,6 +166,7 @@ Status ExchangeSinkBuffer::add_block(TransmitInfo&& request) {
            RETURN_IF_ERROR(
                    BeExecVersionManager::check_be_exec_version(request.block->be_exec_version()));
        }
+        _parent->memory_used_counter()->update(request.block->ByteSizeLong());


我们 exchange sink operator：

先只统计非broad cast 方式的内存，统计方式跟现在这个PR的代码一样。

channel 内保存的那一个block的内存。 broadcast方式的正好没有。

对于broadcast 方式的内存，可能是通过那个holder，构造和析构函数里计算，或者直接使用holder limiter的那个值。

jacktengg · 2024-10-09T17:46:18Z

run buildall

doris-robot · 2024-10-09T19:18:06Z

TeamCity be ut coverage result:
Function Coverage: 37.29% (9636/25838)
Line Coverage: 28.67% (79883/278608)
Region Coverage: 28.08% (41312/147110)
Branch Coverage: 24.70% (21037/85154)
Coverage Report: http://coverage.selectdb-in.cc/coverage/1255d5a76e37f24d0201d9497666cf97c1bc00c4_1255d5a76e37f24d0201d9497666cf97c1bc00c4/report/index.html

jacktengg · 2024-10-10T01:28:36Z

run buildall

doris-robot · 2024-10-10T02:06:09Z

TeamCity be ut coverage result:
Function Coverage: 37.29% (9636/25838)
Line Coverage: 28.69% (79921/278609)
Region Coverage: 28.08% (41314/147111)
Branch Coverage: 24.71% (21041/85154)
Coverage Report: http://coverage.selectdb-in.cc/coverage/ac364bb5ecbb954f335bb8c10cb0b54d590120a9_ac364bb5ecbb954f335bb8c10cb0b54d590120a9/report/index.html

jacktengg · 2024-10-10T02:55:55Z

run p0

yiguolei · 2024-10-10T04:51:10Z

be/src/pipeline/exec/aggregation_sink_operator.cpp

-    Base::_mem_tracker->consume(arena_memory_usage);
-    _serialize_key_arena_memory_usage->add(arena_memory_usage);
-    Base::_shared_state->mem_usage_record.used_in_arena = _agg_arena_pool->size();
+    int64_t arena_memory_usage = _agg_arena_pool->size();


lost agg hash table?

yiguolei · 2024-10-10T04:52:11Z

be/src/pipeline/exec/aggregation_sink_operator.h

@@ -109,7 +109,7 @@ class AggSinkLocalState : public PipelineXSinkLocalState<AggSharedState> {
    RuntimeProfile::Counter* _max_row_size_counter = nullptr;
    RuntimeProfile::Counter* _hash_table_memory_usage = nullptr;


This counter is not updated?

yiguolei · 2024-10-10T04:55:31Z

be/src/pipeline/exec/exchange_sink_operator.cpp

@@ -458,8 +455,11 @@ Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block
        auto rows = block->rows();
        {
            SCOPED_TIMER(local_state._split_block_hash_compute_timer);
-            RETURN_IF_ERROR(
-                    local_state._partitioner->do_partitioning(state, block, _mem_tracker.get()));
+            RETURN_IF_ERROR(local_state._partitioner->do_partitioning(state, block, nullptr));


why add a nullptr not remove this parameter?

yiguolei · 2024-10-10T04:57:40Z

be/src/pipeline/exec/operator.cpp

@@ -345,9 +345,8 @@ void PipelineXLocalStateBase::reached_limit(vectorized::Block* block, bool* eos)
    });

    if (auto rows = block->rows()) {
-        _num_rows_returned += rows;
+        COUNTER_UPDATE(_rows_returned_counter, rows);


这里统一给加了，然后每个operator 内部也在加，难道没有错吗？

yiguolei · 2024-10-10T04:59:35Z

be/src/pipeline/exec/aggregation_source_operator.cpp

@@ -460,18 +460,16 @@ void AggLocalState::do_agg_limit(vectorized::Block* block, bool* eos) {
        if (_shared_state->do_sort_limit && _shared_state->do_limit_filter(block, block->rows())) {
            vectorized::Block::filter_block_internal(block, _shared_state->need_computes);
            if (auto rows = block->rows()) {
-                _num_rows_returned += rows;
+                add_num_rows_returned(rows);


这里也加，然后base 里也加？

yiguolei · 2024-10-10T05:00:02Z

be/src/pipeline/exec/partitioned_hash_join_sink_operator.cpp

@@ -161,7 +161,7 @@ Status PartitionedHashJoinSinkLocalState::_revoke_unpartitioned_block(RuntimeSta

            {
                SCOPED_TIMER(_partition_timer);
-                (void)_partitioner->do_partitioning(state, &sub_block, _mem_tracker.get());
+                (void)_partitioner->do_partitioning(state, &sub_block, nullptr);


这里的nullptr 有啥意义吗？

github-actions · 2024-10-10T08:45:28Z

clang-tidy review says "All clean, LGTM! 👍"

yiguolei

LGTM

jacktengg · 2024-10-10T08:46:10Z

run buildall

github-actions · 2024-10-10T08:46:29Z

PR approved by at least one committer and no changes requested.

github-actions · 2024-10-10T08:46:32Z

PR approved by anyone and no changes requested.

github-actions · 2024-10-10T08:49:06Z

clang-tidy review says "All clean, LGTM! 👍"

doris-robot · 2024-10-10T10:11:19Z

TeamCity be ut coverage result:
Function Coverage: 37.40% (9659/25827)
Line Coverage: 28.67% (80126/279478)
Region Coverage: 28.09% (41425/147451)
Branch Coverage: 24.70% (21099/85418)
Coverage Report: http://coverage.selectdb-in.cc/coverage/f4bd3d8ff1bfbe778bd3aa4facf3cb924bae4669_f4bd3d8ff1bfbe778bd3aa4facf3cb924bae4669/report/index.html

jacktengg · 2024-10-10T14:00:25Z

run buildall

doris-robot · 2024-10-10T14:47:08Z

TeamCity be ut coverage result:
Function Coverage: 37.41% (9661/25827)
Line Coverage: 28.68% (80168/279478)
Region Coverage: 28.10% (41439/147449)
Branch Coverage: 24.71% (21109/85416)
Coverage Report: http://coverage.selectdb-in.cc/coverage/f4bd3d8ff1bfbe778bd3aa4facf3cb924bae4669_f4bd3d8ff1bfbe778bd3aa4facf3cb924bae4669/report/index.html

jacktengg force-pushed the fix-1009-counters branch from 5021271 to 5fa6ca1 Compare October 9, 2024 08:05

github-actions bot reviewed Oct 9, 2024

View reviewed changes

jacktengg force-pushed the fix-1009-counters branch 2 times, most recently from 69f6048 to 3fe3a78 Compare October 9, 2024 08:57

yiguolei reviewed Oct 9, 2024

View reviewed changes

be/src/pipeline/exec/exchange_sink_operator.cpp Outdated Show resolved Hide resolved

yiguolei reviewed Oct 9, 2024

View reviewed changes

jacktengg force-pushed the fix-1009-counters branch 3 times, most recently from 0b594f4 to 1255d5a Compare October 9, 2024 17:45

jacktengg force-pushed the fix-1009-counters branch 2 times, most recently from 275528a to ac364bb Compare October 10, 2024 01:28

yiguolei reviewed Oct 10, 2024

View reviewed changes

jacktengg force-pushed the fix-1009-counters branch from ac364bb to d9ef85e Compare October 10, 2024 08:36

[fix](counters) fix MemoryUsage and PeakMemoryUsage counters

f4bd3d8

jacktengg force-pushed the fix-1009-counters branch from d9ef85e to f4bd3d8 Compare October 10, 2024 08:40

yiguolei approved these changes Oct 10, 2024

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Oct 10, 2024

github-actions bot added the reviewed label Oct 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix](counters) fix MemoryUsage and PeakMemoryUsage counters of some operators #41602

[fix](counters) fix MemoryUsage and PeakMemoryUsage counters of some operators #41602

jacktengg commented Oct 9, 2024

doris-robot commented Oct 9, 2024

jacktengg commented Oct 9, 2024

github-actions bot left a comment

github-actions bot Oct 9, 2024

jacktengg commented Oct 9, 2024

doris-robot commented Oct 9, 2024

yiguolei Oct 9, 2024

jacktengg Oct 9, 2024 •

edited

Loading

yiguolei Oct 9, 2024

yiguolei Oct 9, 2024

yiguolei Oct 9, 2024

jacktengg commented Oct 9, 2024

doris-robot commented Oct 9, 2024

jacktengg commented Oct 10, 2024

doris-robot commented Oct 10, 2024

jacktengg commented Oct 10, 2024

yiguolei Oct 10, 2024

yiguolei Oct 10, 2024

yiguolei Oct 10, 2024

yiguolei Oct 10, 2024

yiguolei Oct 10, 2024

yiguolei Oct 10, 2024

github-actions bot commented Oct 10, 2024

yiguolei left a comment

jacktengg commented Oct 10, 2024

github-actions bot commented Oct 10, 2024

github-actions bot commented Oct 10, 2024

github-actions bot commented Oct 10, 2024

doris-robot commented Oct 10, 2024

jacktengg commented Oct 10, 2024

doris-robot commented Oct 10, 2024

		@@ -109,7 +109,7 @@ class AggSinkLocalState : public PipelineXSinkLocalState<AggSharedState> {
		RuntimeProfile::Counter* _max_row_size_counter = nullptr;
		RuntimeProfile::Counter* _hash_table_memory_usage = nullptr;

[fix](counters) fix MemoryUsage and PeakMemoryUsage counters of some operators #41602

Are you sure you want to change the base?

[fix](counters) fix MemoryUsage and PeakMemoryUsage counters of some operators #41602

Conversation

jacktengg commented Oct 9, 2024

Proposed changes

doris-robot commented Oct 9, 2024

jacktengg commented Oct 9, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

jacktengg commented Oct 9, 2024

doris-robot commented Oct 9, 2024

Choose a reason for hiding this comment

jacktengg Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacktengg commented Oct 9, 2024

doris-robot commented Oct 9, 2024

jacktengg commented Oct 10, 2024

doris-robot commented Oct 10, 2024

jacktengg commented Oct 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 10, 2024

yiguolei left a comment

Choose a reason for hiding this comment

jacktengg commented Oct 10, 2024

github-actions bot commented Oct 10, 2024

github-actions bot commented Oct 10, 2024

github-actions bot commented Oct 10, 2024

doris-robot commented Oct 10, 2024

jacktengg commented Oct 10, 2024

doris-robot commented Oct 10, 2024

jacktengg Oct 9, 2024 •

edited

Loading