vagetablechicken commented on a change in pull request #3286: unregister
fragment mem tracker in close()
URL: https://github.com/apache/incubator-doris/pull/3286#discussion_r406581630
##########
File path: be/src/runtime/plan_fragment_executor.cpp
##########
@@ -556,10 +556,12 @@ void PlanFragmentExecutor::close() {
}
}
- // _mem_tracker init failed
+ // fragment mem tracker needs unregister
if (_mem_tracker.get() != nullptr) {
- _mem_tracker->release(_mem_tracker->consumption());
+ _mem_tracker->unregister_from_parent();
+ _mem_tracker->close();
Review comment:
I searched all usages of `_runtime_state->fragment_mem_tracker()`. Where
else?
How about move this piece to PlanFragmentExecutor dtor? I think it's a
better place.
> By the way, this tracker can be substituted by other tracker. Maybe it can
be done in another issue.
Can't agree more. fragment mem tracker's label is "fragment mem-limit", it's
not very useful. And one plan has two level1 nodes, I think it's not a good
design.
Let's create a new issue to discuss it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
