github-actions[bot] commented on code in PR #67845:
URL: https://github.com/apache/doris/pull/67845#discussion_r4227156566


##########
be/src/runtime/workload_group/workload_group_manager.cpp:
##########
@@ -678,33 +702,38 @@ int64_t 
WorkloadGroupMgr::revoke_memory_from_other_groups_() {
                 // then not revoke memory from it.
                 continue;
             }
-            if (total_used_memory - min_memory_limit > max_exceeded_memory) {
-                max_wg = workload_group.second;
-                max_exceeded_memory = total_used_memory - min_memory_limit;
-            }
+            exceeded_wgs.emplace_back(total_used_memory - min_memory_limit, 
workload_group.second);
         }
     }
-    if (max_wg == nullptr) {
-        return 0;
-    }
-    if (max_exceeded_memory < 1 << 27) {
-        LOG(INFO) << "The workload group that exceed most memory is :"
-                  << max_wg->memory_debug_string() << ", max_exceeded_memory: "
-                  << PrettyPrinter::print(max_exceeded_memory, TUnit::BYTES)
-                  << " less than 128MB, no need to revoke memory";
-        return 0;
-    }
-    int64_t freed_mem = static_cast<int64_t>((double)max_exceeded_memory * 
0.1);
-    // Revoke 10% of memory from the workload group that exceed most memory
-    max_wg->revoke_memory(freed_mem, "exceed_memory", profile.get());
-    std::stringstream ss;
-    profile->pretty_print(&ss);
-    LOG(INFO) << fmt::format(
-            "[MemoryGC] process memory not enough, revoke memory from 
workload_group: {}, "
-            "free memory {}. cost(us): {}, details: {}",
-            max_wg->memory_debug_string(), 
PrettyPrinter::print_bytes(freed_mem),
-            watch.elapsed_time() / 1000, ss.str());
-    return freed_mem;
+    std::sort(exceeded_wgs.begin(), exceeded_wgs.end(),
+              [](const auto& lhs, const auto& rhs) { return lhs.first > 
rhs.first; });
+
+    for (const auto& [exceeded_memory, wg] : exceeded_wgs) {
+        if (exceeded_memory < 1 << 27) {
+            // The remaining workload groups exceed even less.
+            LOG(INFO) << "The workload group that exceed most memory among the 
untried ones is :"
+                      << wg->memory_debug_string() << ", exceeded_memory: "
+                      << PrettyPrinter::print(exceeded_memory, TUnit::BYTES)
+                      << " less than 128MB, no need to revoke memory";
+            break;
+        }
+        auto need_free_mem = static_cast<int64_t>((double)exceeded_memory * 
0.1);
+        // Revoke 10% of memory from the workload group that exceed most memory
+        int64_t freed_mem = wg->revoke_memory(need_free_mem, "exceed_memory", 
profile.get());
+        std::stringstream ss;
+        profile->pretty_print(&ss);

Review Comment:
   [P2] Print the peer reclamation profile once per walk. Each unsuccessful 
`wg->revoke_memory()` appends a child to this same `profile`, and 
`pretty_print()` recursively formats every child. When many over-minimum groups 
hold only uncancellable small queries, this line formats 1+2+...+N children and 
emits that much INFO text on each 50 ms maintenance pass while 
`_paused_queries_lock` is held. That delays admitting and processing paused 
queries precisely during pressure. Print only the current group profile here, 
or format the accumulated profile once after the walk.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to