[GitHub] [incubator-doris] morningman commented on a diff in pull request #10072: [fix] (mem tracker) Fix some memory leaks, inaccurate statistics, core dump, deadlock bugs

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

morningman commented on code in PR #10072:
URL: https://github.com/apache/incubator-doris/pull/10072#discussion_r895192681


##########
be/src/runtime/mem_tracker_task_pool.cpp:
##########
@@ -86,13 +92,22 @@ void MemTrackerTaskPool::logout_task_mem_tracker() {
             it->second->parent()->consume_local(-it->second->consumption(),
                                                 MemTracker::get_process_tracker().get());
             expired_tasks.emplace_back(it->first);
+        } else {
+            // Log limit exceeded query tracker.
+            if (it->second->limit_exceeded()) {
+                it->second->mem_limit_exceeded(
+                        nullptr,
+                        fmt::format("Task mem limit exceeded but no cancel, queryId:{}", it->first),
+                        0, Status::OK());
+            }
         }
     }
     for (auto tid : expired_tasks) {
         // This means that after all RuntimeState is destructed,
         // there are still task mem trackers that are get or register.
         // The only known case: after an load task ends all fragments on a BE,`tablet_writer_open` is still
         // called to create a channel, and the load task tracker will be re-registered in the channel open.
+        // https://github.com/apache/incubator-doris/issues/9905
         if (_task_mem_trackers[tid].use_count() == 1) {

Review Comment:
   `_task_mem_trackers[tid]` maybe nullptr here?
   Because you add a case `if (!it->second)` before.



##########
be/src/runtime/mem_tracker.cpp:
##########
@@ -124,14 +124,17 @@ std::shared_ptr<MemTracker> MemTracker::create_tracker_impl(
     std::string reset_label;
     MemTracker* task_parent_tracker = reset_parent->parent_task_mem_tracker();
     if (task_parent_tracker) {
-        reset_label = fmt::format("{}:{}", label, split(task_parent_tracker->label(), ":")[1]);
+        reset_label = fmt::format("{}&{}", label, split(task_parent_tracker->label(), "&")[1]);

Review Comment:
   Why change to `&`?



-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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