xinyiZzz commented on code in PR #47462:
URL: https://github.com/apache/doris/pull/47462#discussion_r1966807381


##########
be/src/runtime/query_context.cpp:
##########
@@ -146,34 +151,45 @@ QueryContext::QueryContext(TUniqueId query_id, ExecEnv* 
exec_env,
 
 void QueryContext::_init_query_mem_tracker() {
     bool has_query_mem_limit = _query_options.__isset.mem_limit && 
(_query_options.mem_limit > 0);
-    int64_t _bytes_limit = has_query_mem_limit ? _query_options.mem_limit : -1;
-    if (_bytes_limit > MemInfo::mem_limit()) {
-        VLOG_NOTICE << "Query memory limit " << 
PrettyPrinter::print(_bytes_limit, TUnit::BYTES)
+    int64_t bytes_limit = has_query_mem_limit ? _query_options.mem_limit : -1;
+    if (bytes_limit > MemInfo::mem_limit() || bytes_limit == -1) {
+        VLOG_NOTICE << "Query memory limit " << 
PrettyPrinter::print(bytes_limit, TUnit::BYTES)
                     << " exceeds process memory limit of "
                     << PrettyPrinter::print(MemInfo::mem_limit(), TUnit::BYTES)
-                    << ". Using process memory limit instead";
-        _bytes_limit = MemInfo::mem_limit();
+                    << " OR is -1. Using process memory limit instead.";
+        bytes_limit = MemInfo::mem_limit();
+    }
+    // If the query is a pure load task(streamload, routine load, group 
commit), then it should not use
+    // memlimit per query to limit their memory usage.
+    if (is_pure_load_task()) {

Review Comment:
   现在也不支持限制单个 Load 任务的 memtable 内存大小,那限制 Load 任务的内存只能用 Workload Group 了吧
   用户线上经常出现各种类型的导入任务把内存打满。



##########
be/src/runtime/thread_context.h:
##########
@@ -223,6 +224,21 @@ class ThreadContext {
     // to nullptr, but the object it points to is not initialized. At this 
time, when the memory
     // is released somewhere, the hook is triggered to cause the crash.
     std::unique_ptr<ThreadMemTrackerMgr> thread_mem_tracker_mgr;
+
+    [[nodiscard]] std::shared_ptr<MemTrackerLimiter> thread_mem_tracker() 
const {
+        return thread_mem_tracker_mgr->limiter_mem_tracker();

Review Comment:
   之所以让 thread_mem_tracker_mgr 作为 public 属性,就是不想在 ThreadContext 
中新增方法,现在外部都是直接用的  
`thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker()`。 目的是尽量让 
ThreadContext 这个类简单,后续不再修改,不过这个设计模式也值得商榷。



##########
be/src/runtime/workload_management/io_context.h:
##########
@@ -58,6 +61,10 @@ class IOContext : public 
std::enable_shared_from_this<IOContext> {
             shuffle_send_bytes_counter_ = ADD_COUNTER(profile_, 
"ShuffleSendBytes", TUnit::BYTES);
             shuffle_send_rows_counter_ =
                     ADD_COUNTER(profile_, "ShuffleSendRowsCounter_", 
TUnit::UNIT);
+            spill_write_bytes_to_local_storage_counter_ =
+                    ADD_COUNTER(profile_, "SpillWriteBytesToLocalStorage", 
TUnit::UNIT);

Review Comment:
   单位应该是 TUnit::BYTES



##########
be/src/runtime/runtime_state.h:
##########
@@ -605,7 +643,16 @@ class RuntimeState {
         if (_query_options.__isset.min_revocable_mem) {
             return std::max(_query_options.min_revocable_mem, (int64_t)1);
         }
-        return 1;
+        return 32L * 1024 * 1024;
+    }
+
+    size_t minimum_operator_memory_required_bytes() const {
+        if (_query_options.__isset.minimum_operator_memory_required_kb) {

Review Comment:
   除了 join、agg、sort 外的其他算子,reserve memory 
也要检查这个最小值是否满足么,这会不会增加复杂度啊,只让这3个算子检查是不是就行



##########
be/src/runtime/thread_context.h:
##########
@@ -223,6 +224,21 @@ class ThreadContext {
     // to nullptr, but the object it points to is not initialized. At this 
time, when the memory
     // is released somewhere, the hook is triggered to cause the crash.
     std::unique_ptr<ThreadMemTrackerMgr> thread_mem_tracker_mgr;
+
+    [[nodiscard]] std::shared_ptr<MemTrackerLimiter> thread_mem_tracker() 
const {
+        return thread_mem_tracker_mgr->limiter_mem_tracker();
+    }
+
+    doris::Status try_reserve_process_memory(const int64_t size) const {
+        return thread_mem_tracker_mgr->try_reserve(size, true);
+    }
+
+    doris::Status try_reserve_memory(const int64_t size) const {
+        return thread_mem_tracker_mgr->try_reserve(size, false);
+    }
+
+    void release_reserved_memory() const { 
thread_mem_tracker_mgr->shrink_reserved(); }

Review Comment:
   我们统一叫 shrink_reserved_memory 吧



##########
be/src/runtime/workload_management/io_context.h:
##########
@@ -58,6 +61,10 @@ class IOContext : public 
std::enable_shared_from_this<IOContext> {
             shuffle_send_bytes_counter_ = ADD_COUNTER(profile_, 
"ShuffleSendBytes", TUnit::BYTES);
             shuffle_send_rows_counter_ =
                     ADD_COUNTER(profile_, "ShuffleSendRowsCounter_", 
TUnit::UNIT);
+            spill_write_bytes_to_local_storage_counter_ =
+                    ADD_COUNTER(profile_, "SpillWriteBytesToLocalStorage", 
TUnit::UNIT);
+            spill_read_bytes_from_local_storage_counter_ =
+                    ADD_COUNTER(profile_, "SpillReadBytesFromLocalStorage", 
TUnit::UNIT);

Review Comment:
   同上,单位应该是 TUnit::BYTES



-- 
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