You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "xinyiZzz (via GitHub)" <gi...@apache.org> on 2023/04/12 08:13:19 UTC

[GitHub] [doris] xinyiZzz opened a new pull request, #18590: [enhancement](memory) Optimize memory limit exceeded behavior

xinyiZzz opened a new pull request, #18590:
URL: https://github.com/apache/doris/pull/18590

   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem summary
   
   1. No longer cancel query/load in mem hook, only cancel in Allocator. 
      only PODArray/hash table/arena memory allocation will use Allocator.
   
   2. Optimize mem limit exceeded log printing
   
   ## Checklist(Required)
   
   * [ ] Does it affect the original behavior
   * [ ] Has unit tests been added
   * [ ] Has document been added or modified
   * [ ] Does it need to update dependencies
   * [ ] Is this PR support rollback (If NO, please explain WHY)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   
   


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


[GitHub] [doris] xinyiZzz commented on pull request #18590: [enhancement](memory) Optimize memory limit exceeded behavior

Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on PR #18590:
URL: https://github.com/apache/doris/pull/18590#issuecomment-1504864594

   run buildall


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


[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18590: [enhancement](memory) Refactor memory limit exceeded behavior

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18590:
URL: https://github.com/apache/doris/pull/18590#discussion_r1165570391


##########
be/src/runtime/memory/thread_mem_tracker_mgr.h:
##########
@@ -21,21 +21,20 @@
 #include <fmt/format.h>
 
 #include "gutil/macros.h"
+#include "runtime/exec_env.h"
 #include "runtime/memory/mem_tracker.h"
 #include "runtime/memory/mem_tracker_limiter.h"
 
 namespace doris {
 
-using ExceedCallBack = void (*)();
-
 // Memory Hook is counted in the memory tracker of the current thread.
 class ThreadMemTrackerMgr {
 public:
     ThreadMemTrackerMgr() {}
 
     ~ThreadMemTrackerMgr() {
         // if _init == false, exec env is not initialized when init(). and never consumed mem tracker once.
-        if (_init) flush_untracked_mem<false, true>();
+        if (_init) flush_untracked_mem();

Review Comment:
   warning: statement should be inside braces [readability-braces-around-statements]
   
   ```suggestion
           if (_init) { flush_untracked_mem();
   }
   ```
   



##########
be/src/runtime/thread_context.h:
##########
@@ -35,6 +35,7 @@
 // Usage example: int64_t scope_mem = 0; { SCOPED_MEM_COUNT(&scope_mem); xxx; xxx; }
 #define SCOPED_MEM_COUNT(scope_mem) \

Review Comment:
   warning: macro is not used [clang-diagnostic-unused-macros]
   ```cpp
   #define SCOPED_MEM_COUNT(scope_mem) \
           ^
   ```
   



##########
be/src/runtime/thread_context.h:
##########
@@ -56,29 +57,15 @@
 // Usage is similar to SCOPED_CONSUME_MEM_TRACKER.
 #define SCOPED_ATTACH_TASK(arg1, ...) \

Review Comment:
   warning: macro is not used [clang-diagnostic-unused-macros]
   ```cpp
   #define SCOPED_ATTACH_TASK(arg1, ...) \
           ^
   ```
   



##########
be/src/runtime/thread_context.h:
##########
@@ -56,29 +57,15 @@
 // Usage is similar to SCOPED_CONSUME_MEM_TRACKER.
 #define SCOPED_ATTACH_TASK(arg1, ...) \
     auto VARNAME_LINENUM(attach_task) = AttachTask(arg1, ##__VA_ARGS__)
+
 // Switch MemTrackerLimiter for count memory during thread execution.
 // Usually used after SCOPED_ATTACH_TASK, in order to count the memory of the specified code segment into another
 // MemTrackerLimiter instead of the MemTrackerLimiter added by the attach task.
 #define SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(mem_tracker_limiter) \

Review Comment:
   warning: macro is not used [clang-diagnostic-unused-macros]
   ```cpp
   #define SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(mem_tracker_limiter) \
           ^
   ```
   



##########
be/src/runtime/thread_context.h:
##########
@@ -279,28 +276,11 @@
     bool _need_pop = false;
 };
 
-class StopCheckThreadMemTrackerLimit {
-public:
-    explicit StopCheckThreadMemTrackerLimit() {
-        _pre = thread_context()->thread_mem_tracker_mgr->check_limit();
-        thread_context()->thread_mem_tracker_mgr->set_check_limit(false);
-    }
-
-    ~StopCheckThreadMemTrackerLimit() {
-        thread_context()->thread_mem_tracker_mgr->set_check_limit(_pre);
-    }
-
-private:
-    bool _pre;
-};
-
 // Basic macros for mem tracker, usually do not need to be modified and used.
 #ifdef USE_MEM_TRACKER
 // For the memory that cannot be counted by mem hook, manually count it into the mem tracker, such as mmap.
 #define CONSUME_THREAD_MEM_TRACKER(size) \

Review Comment:
   warning: macro is not used [clang-diagnostic-unused-macros]
   ```cpp
   #define CONSUME_THREAD_MEM_TRACKER(size) \
           ^
   ```
   



##########
be/src/vec/common/allocator.cpp:
##########
@@ -0,0 +1,120 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "vec/common/allocator.h"
+
+// Allocator is used by too many files. For compilation speed, put dependencies in `.cpp` as much as possible.
+#include "runtime/thread_context.h"
+#include "util/mem_info.h"
+
+template <bool clear_memory_, bool mmap_populate>
+void Allocator<clear_memory_, mmap_populate>::sys_memory_check(size_t size) const {
+    if (doris::MemTrackerLimiter::sys_mem_exceed_limit_check(size)) {
+        // Only thread attach query, and has not completely waited for thread_wait_gc_max_milliseconds,
+        // will wait for gc, asynchronous cancel or throw bad::alloc.
+        // Otherwise, if the external catch, directly throw bad::alloc.
+        if (doris::thread_context()->thread_mem_tracker_mgr->is_attach_query() &&
+            doris::thread_context()->thread_mem_tracker_mgr->wait_gc()) {
+            int64_t wait_milliseconds = doris::config::thread_wait_gc_max_milliseconds;
+            while (wait_milliseconds > 0) {
+                std::this_thread::sleep_for(std::chrono::milliseconds(100));
+                if (!doris::MemTrackerLimiter::sys_mem_exceed_limit_check(size)) {
+                    doris::MemInfo::refresh_interval_memory_growth += size;
+                    break;
+                }
+                wait_milliseconds -= 100;
+            }
+            if (wait_milliseconds <= 0) {
+                // Make sure to completely wait thread_wait_gc_max_milliseconds only once.
+                doris::thread_context()->thread_mem_tracker_mgr->disable_wait_gc();
+                auto err_msg = fmt::format(
+                        "Allocator sys memory check failed: Cannot alloc:{}, consuming "
+                        "tracker:<{}>, exec node:<{}>, {}.",
+                        size, doris::thread_context()->thread_mem_tracker()->label(),
+                        doris::thread_context()->thread_mem_tracker_mgr->last_consumer_tracker(),
+                        doris::MemTrackerLimiter::process_limit_exceeded_errmsg_str(size));
+                // If the external catch, throw bad::alloc first, let the query actively cancel. Otherwise asynchronous cancel.
+                if (!doris::enable_thread_catch_bad_alloc) {
+                    doris::thread_context()->thread_mem_tracker_mgr->cancel_fragment(err_msg);
+                } else {
+                    doris::thread_context()->thread_mem_tracker_mgr->save_exceed_mem_limit_msg(
+                            err_msg);
+                    throw std::bad_alloc {};
+                }
+            }
+        } else if (doris::enable_thread_catch_bad_alloc) {
+            auto err_msg = fmt::format(
+                    "Allocator sys memory check failed: Cannot alloc:{}, consuming tracker:<{}>, "
+                    "exec node:<{}>, {}.",
+                    size, doris::thread_context()->thread_mem_tracker()->label(),
+                    doris::thread_context()->thread_mem_tracker_mgr->last_consumer_tracker(),
+                    doris::MemTrackerLimiter::process_limit_exceeded_errmsg_str(size));
+            doris::thread_context()->thread_mem_tracker_mgr->save_exceed_mem_limit_msg(err_msg);
+            throw std::bad_alloc {};
+        }
+    }
+}
+
+template <bool clear_memory_, bool mmap_populate>
+void Allocator<clear_memory_, mmap_populate>::memory_tracker_check(size_t size) const {
+    auto st = doris::thread_context()->thread_mem_tracker()->check_limit(size);
+    if (!st) {
+        doris::thread_context()->thread_mem_tracker_mgr->disable_wait_gc();
+        auto err_msg =
+                doris::thread_context()->thread_mem_tracker()->query_tracker_limit_exceeded_str(
+                        st.to_string(),
+                        doris::thread_context()->thread_mem_tracker_mgr->last_consumer_tracker(),
+                        "Allocator mem tracker check failed");
+        // If the external catch, throw bad::alloc first, let the query actively cancel. Otherwise asynchronous cancel.
+        if (!doris::enable_thread_catch_bad_alloc) {
+            doris::thread_context()->thread_mem_tracker_mgr->cancel_fragment(err_msg);
+            doris::thread_context()->thread_mem_tracker()->print_log_usage(err_msg);
+        } else {
+            doris::thread_context()->thread_mem_tracker_mgr->save_exceed_mem_limit_msg(err_msg);
+            throw std::bad_alloc {};
+        }
+    }
+}
+
+template <bool clear_memory_, bool mmap_populate>
+void Allocator<clear_memory_, mmap_populate>::memory_check(size_t size) const {
+    sys_memory_check(size);
+    memory_tracker_check(size);
+}
+
+template <bool clear_memory_, bool mmap_populate>
+void Allocator<clear_memory_, mmap_populate>::consume_memory(size_t size) const {
+    CONSUME_THREAD_MEM_TRACKER(size);
+}
+
+template <bool clear_memory_, bool mmap_populate>
+void Allocator<clear_memory_, mmap_populate>::release_memory(size_t size) const {
+    RELEASE_THREAD_MEM_TRACKER(size);
+}
+
+template <bool clear_memory_, bool mmap_populate>
+void Allocator<clear_memory_, mmap_populate>::throw_bad_alloc(const std::string& err) const {
+    LOG(WARNING) << err;
+    if (!doris::enable_thread_catch_bad_alloc)
+        doris::MemTrackerLimiter::print_log_process_usage(err);

Review Comment:
   warning: statement should be inside braces [readability-braces-around-statements]
   
   ```suggestion
       if (!doris::enable_thread_catch_bad_alloc) {
           doris::MemTrackerLimiter::print_log_process_usage(err);
   }
   ```
   



##########
be/src/vec/common/allocator.h:
##########
@@ -29,7 +29,6 @@
 #include "common/status.h"
 #include "runtime/memory/chunk.h"
 #include "runtime/memory/chunk_allocator.h"
-#include "runtime/thread_context.h"
 
 #ifdef NDEBUG
 #define ALLOCATOR_ASLR 0

Review Comment:
   warning: macro is not used [clang-diagnostic-unused-macros]
   ```cpp
   #define ALLOCATOR_ASLR 0
           ^
   ```
   



##########
be/src/runtime/thread_context.h:
##########
@@ -354,22 +334,6 @@
             doris::ExecEnv::GetInstance()->orphan_mem_tracker_raw()->consume_no_update_peak(size); \
         }                                                                                          \
     } while (0)
-// NOTE, The LOG cannot be printed in the mem hook. If the LOG statement triggers the mem hook LOG,
-// the nested LOG may cause an unknown crash.
-#define TRY_CONSUME_MEM_TRACKER(size, fail_ret)                                                    \
-    do {                                                                                           \
-        if (doris::thread_context_ptr.init) {                                                      \
-            if (doris::enable_thread_catch_bad_alloc) {                                            \
-                if (!doris::thread_context()->thread_mem_tracker_mgr->try_consume(size)) {         \
-                    return fail_ret;                                                               \
-                }                                                                                  \
-            } else {                                                                               \
-                doris::thread_context()->thread_mem_tracker_mgr->consume(size);                    \
-            }                                                                                      \
-        } else if (doris::ExecEnv::GetInstance()->initialized()) {                                 \
-            doris::ExecEnv::GetInstance()->orphan_mem_tracker_raw()->consume_no_update_peak(size); \
-        }                                                                                          \
-    } while (0)
 #define RELEASE_MEM_TRACKER(size)                                                            \

Review Comment:
   warning: macro is not used [clang-diagnostic-unused-macros]
   ```cpp
   #define RELEASE_MEM_TRACKER(size)                                                            \
           ^
   ```
   



##########
be/src/runtime/thread_context.h:
##########
@@ -279,28 +276,11 @@
     bool _need_pop = false;
 };
 
-class StopCheckThreadMemTrackerLimit {
-public:
-    explicit StopCheckThreadMemTrackerLimit() {
-        _pre = thread_context()->thread_mem_tracker_mgr->check_limit();
-        thread_context()->thread_mem_tracker_mgr->set_check_limit(false);
-    }
-
-    ~StopCheckThreadMemTrackerLimit() {
-        thread_context()->thread_mem_tracker_mgr->set_check_limit(_pre);
-    }
-
-private:
-    bool _pre;
-};
-
 // Basic macros for mem tracker, usually do not need to be modified and used.
 #ifdef USE_MEM_TRACKER
 // For the memory that cannot be counted by mem hook, manually count it into the mem tracker, such as mmap.
 #define CONSUME_THREAD_MEM_TRACKER(size) \
     doris::thread_context()->thread_mem_tracker_mgr->consume(size)
-#define TRY_CONSUME_THREAD_MEM_TRACKER(size) \
-    doris::thread_context()->thread_mem_tracker_mgr->try_consume(size)
 #define RELEASE_THREAD_MEM_TRACKER(size) \

Review Comment:
   warning: macro is not used [clang-diagnostic-unused-macros]
   ```cpp
   #define RELEASE_THREAD_MEM_TRACKER(size) \
           ^
   ```
   



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


[GitHub] [doris] yiguolei merged pull request #18590: [enhancement](memory) Refactor memory limit exceeded behavior

Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #18590:
URL: https://github.com/apache/doris/pull/18590


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


[GitHub] [doris] xinyiZzz commented on pull request #18590: [enhancement](memory) Refactor memory limit exceeded behavior

Posted by "xinyiZzz (via GitHub)" <gi...@apache.org>.
xinyiZzz commented on PR #18590:
URL: https://github.com/apache/doris/pull/18590#issuecomment-1507032355

   run buildall


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


[GitHub] [doris] github-actions[bot] commented on pull request #18590: [enhancement](memory) Optimize memory limit exceeded behavior

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18590:
URL: https://github.com/apache/doris/pull/18590#issuecomment-1504858694

   clang-tidy review says "All clean, LGTM! :+1:"


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