[GitHub] [incubator-doris] imay commented on a change in pull request #3286: unregister fragment mem tracker in close()

[GitBox <gi...@apache.org>]

imay commented on a change in pull request #3286: unregister fragment mem tracker in close()
URL: https://github.com/apache/incubator-doris/pull/3286#discussion_r405984076
 
 

 ##########
 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:
   Why not keep origin code and just add `unregister_from_parent`.
   If you reset this mem_tracker here, I'm not sure if it will work in some situation, some runtime holder may access this tracker.
   So, just to keep things simple, better not change origin code.
   
   By the way, this tracker can be substituted by other tracker. Maybe it can be done in another issue.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org