From 807499427cd308fb5992bbfa38ff3f451715138d Mon Sep 17 00:00:00 2001 From: HuangWei Date: Mon, 13 Apr 2020 23:15:56 +0800 Subject: [PATCH] unregister fragment mem tracker in close() (#3286) ref https://github.com/apache/incubator-doris/issues/3273 P.S. https://github.com/apache/incubator-doris/blob/614a76beeac73821c78903c46e7a703b7956796b/be/src/runtime/plan_fragment_executor.cpp#L559-L562 I think this piece of code is useless. This `_mem_tracker` in `PlanFragmentExecutor` is set as fragment_mem_tracker of `RuntimeState`. **direct use** We use it in these code, when rowbatch reset, mem tracker's consumption will be released. https://github.com/apache/incubator-doris/blob/7eab12a40e5a20959c13eba99697c171d1461e0b/be/src/exec/olap_rewrite_node.cpp#L57-L58 https://github.com/apache/incubator-doris/blob/839ec45197fb4559c4946bf0fa4ce3b6805b4248/be/src/exec/olap_scan_node.cpp#L1217-L1218 **other usage** e.g. https://github.com/apache/incubator-doris/blob/6c33f805449d9a7bc8809a575407acfa87bb061e/be/src/exec/olap_scanner.cpp#L245 won't consume the fragment mem tracker. We don't need to worry about the fragment mem tracker consumption is not zero when we want to destroy it. Or we can add a consumption check before we close the mem tracker? --- be/src/runtime/plan_fragment_executor.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/be/src/runtime/plan_fragment_executor.cpp b/be/src/runtime/plan_fragment_executor.cpp index 18cfb0d223..0c7c7fb2bd 100644 --- a/be/src/runtime/plan_fragment_executor.cpp +++ b/be/src/runtime/plan_fragment_executor.cpp @@ -65,6 +65,11 @@ PlanFragmentExecutor::~PlanFragmentExecutor() { // } // at this point, the report thread should have been stopped DCHECK(!_report_thread_active); + + // fragment mem tracker needs unregister + if (_mem_tracker.get() != nullptr) { + _mem_tracker->unregister_from_parent(); + } } Status PlanFragmentExecutor::prepare(const TExecPlanFragmentParams& request) {