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


##########
be/src/runtime/memory/mem_tracker_limiter.cpp:
##########
@@ -240,42 +242,71 @@ Status 
MemTrackerLimiter::fragment_mem_limit_exceeded(RuntimeState* state, const
     return Status::MemoryLimitExceeded(failed_msg);
 }
 
-// TODO(zxy) More observable methods
-// /// Logs the usage of 'limit' number of queries based on maximum total 
memory
-// /// consumption.
-// std::string MemTracker::LogTopNQueries(int limit) {
-//     if (limit == 0) return "";
-//     priority_queue<pair<int64_t, string>, std::vector<pair<int64_t, 
string>>,
-//                    std::greater<pair<int64_t, string>>>
-//             min_pq;
-//     GetTopNQueries(min_pq, limit);
-//     std::vector<string> usage_strings(min_pq.size());
-//     while (!min_pq.empty()) {
-//         usage_strings.push_back(min_pq.top().second);
-//         min_pq.pop();
-//     }
-//     std::reverse(usage_strings.begin(), usage_strings.end());
-//     return join(usage_strings, "\n");
-// }
+void MemTrackerLimiter::free_top_query(int64_t min_free_mem) {
+    std::priority_queue<std::pair<int64_t, std::string>,
+                        std::vector<std::pair<int64_t, std::string>>,
+                        std::greater<std::pair<int64_t, std::string>>>
+            min_pq;
+    int64_t prepare_free_mem = 0;
 
-// /// Helper function for LogTopNQueries that iterates through the MemTracker 
hierarchy
-// /// and populates 'min_pq' with 'limit' number of elements (that contain 
state related
-// /// to query MemTrackers) based on maximum total memory consumption.
-// void MemTracker::GetTopNQueries(
-//         priority_queue<pair<int64_t, string>, std::vector<pair<int64_t, 
string>>,
-//                        greater<pair<int64_t, string>>>& min_pq,
-//         int limit) {
-//     list<weak_ptr<MemTracker>> children;
-//     {
-//         lock_guard<SpinLock> l(child_trackers_lock_);
-//         children = child_trackers_;
-//     }
-//     for (const auto& child_weak : children) {
-//         shared_ptr<MemTracker> child = child_weak.lock();
-//         if (child) {
-//             child->GetTopNQueries(min_pq, limit);
-//         }
-//     }
-// }
+    auto label_to_queryid = [&](const std::string& label) -> TUniqueId {
+        auto queryid = split(label, "#Id=")[1];
+        TUniqueId querytid;
+        parse_id(queryid, &querytid);
+        return querytid;
+    };
+
+    auto cancel_top_query = [&](auto min_pq, auto label_to_queryid) {
+        std::vector<std::string> usage_strings;
+        bool had_cancel = false;
+        while (!min_pq.empty()) {
+            TUniqueId cancelled_queryid = 
label_to_queryid(min_pq.top().second);
+            ExecEnv::GetInstance()->fragment_mgr()->cancel_query(
+                    cancelled_queryid, 
PPlanFragmentCancelReason::MEMORY_LIMIT_EXCEED,
+                    fmt::format("Process has no memory available, cancel top 
memory usage query: "
+                                "query memory tracker <{}> consumption {}, 
backend {} "
+                                "process memory used {} exceed limit {} or sys 
mem available {} "
+                                "less than low water mark {}. Execute again 
after enough memory, "
+                                "details see be.INFO.",
+                                min_pq.top().second, 
print_bytes(min_pq.top().first),
+                                BackendOptions::get_localhost(), 
PerfCounters::get_vm_rss_str(),
+                                MemInfo::mem_limit_str(), 
MemInfo::sys_mem_available_str(),
+                                
print_bytes(MemInfo::sys_mem_available_low_water_mark())));
+
+            usage_strings.push_back(
+                    fmt::format("{} memory usage {}B", min_pq.top().second, 
min_pq.top().first));
+            had_cancel = true;
+            min_pq.pop();
+        }
+        if (had_cancel) LOG(INFO) << "Free Top Memory Usage Query: " << 
join(usage_strings, ",");

Review Comment:
   warning: statement should be inside braces 
[readability-braces-around-statements]
   
   ```suggestion
           if (had_cancel) { LOG(INFO) << "Free Top Memory Usage Query: " << 
join(usage_strings, ",");
   }
   ```
   



##########
be/src/util/mem_info.cpp:
##########
@@ -84,6 +87,24 @@
 #endif
 }
 
+void MemInfo::process_minor_gc() {
+    StoragePageCache::instance()->prune(segment_v2::DATA_PAGE);
+    ChunkAllocator::instance()->clear();
+    // TODO, free more cache etc.
+}
+
+void MemInfo::process_full_gc() {
+    int64_t prepare_free_mem = _s_process_full_gc_size;
+    prepare_free_mem -=
+            
StoragePageCache::instance()->get_page_cache_mem_consumption(segment_v2::DATA_PAGE);
+    StoragePageCache::instance()->prune(segment_v2::DATA_PAGE);
+    if (prepare_free_mem <= 0) return;
+    prepare_free_mem -= ChunkAllocator::instance()->mem_consumption();
+    ChunkAllocator::instance()->clear();
+    if (prepare_free_mem <= 0) return;

Review Comment:
   warning: statement should be inside braces 
[readability-braces-around-statements]
   
   ```suggestion
       if (prepare_free_mem <= 0) { return;
   }
   ```
   



##########
be/src/util/mem_info.cpp:
##########
@@ -84,6 +87,24 @@ void MemInfo::refresh_allocator_mem() {
 #endif
 }
 
+void MemInfo::process_minor_gc() {
+    StoragePageCache::instance()->prune(segment_v2::DATA_PAGE);
+    ChunkAllocator::instance()->clear();
+    // TODO, free more cache etc.
+}
+
+void MemInfo::process_full_gc() {
+    int64_t prepare_free_mem = _s_process_full_gc_size;
+    prepare_free_mem -=
+            
StoragePageCache::instance()->get_page_cache_mem_consumption(segment_v2::DATA_PAGE);
+    StoragePageCache::instance()->prune(segment_v2::DATA_PAGE);
+    if (prepare_free_mem <= 0) return;

Review Comment:
   warning: statement should be inside braces 
[readability-braces-around-statements]
   
   ```suggestion
       if (prepare_free_mem <= 0) { return;
   }
   ```
   



##########
be/src/runtime/memory/chunk_allocator.cpp:
##########
@@ -120,6 +120,17 @@ class ChunkArena {
         _chunk_lists[idx].push_back(ptr);
     }
 
+    void clear() {
+        std::lock_guard<SpinLock> l(_lock);
+        for (int i = 0; i < 64; ++i) {
+            if (_chunk_lists[i].empty()) continue;

Review Comment:
   warning: statement should be inside braces 
[readability-braces-around-statements]
   
   ```suggestion
               if (_chunk_lists[i].empty()) { continue;
   }
   ```
   



##########
be/src/olap/lru_cache.h:
##########
@@ -371,6 +373,8 @@ class ShardedLRUCache : public Cache {
     virtual int64_t prune() override;
     virtual int64_t prune_if(CacheValuePredicate pred) override;
 
+    virtual int64_t mem_consumption() override { return 
_mem_tracker->consumption(); }

Review Comment:
   warning: 'virtual' is redundant since the function is already declared 
'override' [modernize-use-override]
   
   ```suggestion
   if(CacheValuePredicate pred) override;
   ```
   



-- 
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-unsubscr...@doris.apache.org

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


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

Reply via email to