github-actions[bot] commented on code in PR #33140:
URL: https://github.com/apache/doris/pull/33140#discussion_r1546499712
##########
be/src/runtime/group_commit_mgr.cpp:
##########
@@ -228,7 +229,9 @@ Status GroupCommitTable::get_first_block_load_queue(
std::to_string(_table_id));
}
-Status GroupCommitTable::_create_group_commit_load(int be_exe_version) {
+Status GroupCommitTable::_create_group_commit_load(int be_exe_version,
Review Comment:
warning: function '_create_group_commit_load' exceeds recommended
size/complexity thresholds [readability-function-size]
```cpp
Status GroupCommitTable::_create_group_commit_load(int be_exe_version,
^
```
<details>
<summary>Additional context</summary>
**be/src/runtime/group_commit_mgr.cpp:231:** 95 lines including whitespace
and comments (threshold 80)
```cpp
Status GroupCommitTable::_create_group_commit_load(int be_exe_version,
^
```
</details>
##########
be/src/runtime/fragment_mgr.cpp:
##########
@@ -976,6 +979,16 @@ void FragmentMgr::_set_scan_concurrency(const Param&
params, QueryContext* query
#endif
}
+std::shared_ptr<QueryContext> FragmentMgr::get_query_context(const TUniqueId&
query_id) {
Review Comment:
warning: method 'get_query_context' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static std::shared_ptr<QueryContext> FragmentMgr::get_query_context(const
TUniqueId& query_id) {
```
##########
be/src/runtime/query_context.cpp:
##########
@@ -46,48 +47,53 @@ QueryContext::QueryContext(TUniqueId query_id, int
total_fragment_num, ExecEnv*
_is_pipeline(is_pipeline),
_is_nereids(is_nereids),
_query_options(query_options) {
+ _init_query_mem_tracker();
+ SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(query_mem_tracker);
this->coord_addr = coord_addr;
_start_time = VecDateTimeValue::local_time();
_shared_hash_table_controller.reset(new
vectorized::SharedHashTableController());
_shared_scanner_controller.reset(new
vectorized::SharedScannerController());
_execution_dependency =
pipeline::Dependency::create_unique(-1, -1, "ExecutionDependency",
this);
_runtime_filter_mgr = std::make_unique<RuntimeFilterMgr>(
- TUniqueId(), RuntimeFilterParamsContext::create(this));
+ TUniqueId(), RuntimeFilterParamsContext::create(this),
query_mem_tracker);
timeout_second = query_options.execution_timeout;
- bool has_query_mem_tracker = query_options.__isset.mem_limit &&
(query_options.mem_limit > 0);
- int64_t _bytes_limit = has_query_mem_tracker ? query_options.mem_limit :
-1;
+ register_memory_statistics();
+ register_cpu_statistics();
+}
+
+void QueryContext::_init_query_mem_tracker() {
Review Comment:
warning: method '_init_query_mem_tracker' can be made static
[readability-convert-member-functions-to-static]
be/src/runtime/query_context.h:297:
```diff
- void _init_query_mem_tracker();
+ static void _init_query_mem_tracker();
```
##########
be/src/runtime/thread_context.cpp:
##########
@@ -23,20 +23,39 @@
namespace doris {
class MemTracker;
+QueryThreadContext ThreadContext::query_thread_context() {
Review Comment:
warning: method 'query_thread_context' can be made static
[readability-convert-member-functions-to-static]
be/src/runtime/thread_context.h:196:
```diff
- QueryThreadContext query_thread_context();
+ static QueryThreadContext query_thread_context();
```
##########
be/src/runtime/memory/mem_tracker_limiter.cpp:
##########
@@ -83,35 +78,69 @@ MemTrackerLimiter::MemTrackerLimiter(Type type, const
std::string& label, int64_
if (_type == Type::LOAD || _type == Type::QUERY) {
_query_statistics = std::make_shared<QueryStatistics>();
}
-
- {
- std::lock_guard<std::mutex>
l(mem_tracker_limiter_pool[_group_num].group_lock);
- _tracker_limiter_group_it =
mem_tracker_limiter_pool[_group_num].trackers.insert(
- mem_tracker_limiter_pool[_group_num].trackers.end(), this);
- }
g_memtrackerlimiter_cnt << 1;
}
+std::shared_ptr<MemTrackerLimiter>
MemTrackerLimiter::create_shared(MemTrackerLimiter::Type type,
Review Comment:
warning: method 'create_shared' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static std::shared_ptr<MemTrackerLimiter>
MemTrackerLimiter::create_shared(MemTrackerLimiter::Type type,
```
##########
be/src/runtime/thread_context.h:
##########
@@ -259,29 +276,56 @@ static ThreadContext* thread_context() {
DCHECK(thread_context_ptr != nullptr);
return thread_context_ptr;
}
+#if !defined(USE_MEM_TRACKER) || defined(BE_TEST)
+ DCHECK(doris::ExecEnv::ready());
+ return doris::ExecEnv::GetInstance()->env_thread_context();
+#endif
if (bthread_self() != 0) {
// in bthread
// bthread switching pthread may be very frequent, remember not to use
lock or other time-consuming operations.
auto* bthread_context =
static_cast<ThreadContext*>(bthread_getspecific(btls_key));
DCHECK(bthread_context != nullptr);
return bthread_context;
}
- LOG(FATAL) << "__builtin_unreachable";
+ // It means that use thread_context() but this thread not attached a
query/load using SCOPED_ATTACH_TASK macro.
+ LOG(FATAL) << "__builtin_unreachable, " << doris::memory_orphan_check_msg;
__builtin_unreachable();
}
+class QueryThreadContext {
+public:
+ QueryThreadContext() = default;
+ QueryThreadContext(const TUniqueId& query_id,
+ const std::shared_ptr<MemTrackerLimiter>& mem_tracker)
+ : query_id(query_id), query_mem_tracker(mem_tracker) {}
+
+ void init() {
Review Comment:
warning: method 'init' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static void init() {
```
--
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
