github-actions[bot] commented on code in PR #29337:
URL: https://github.com/apache/doris/pull/29337#discussion_r1438477637
##########
be/src/pipeline/task_scheduler.h:
##########
@@ -93,6 +93,10 @@ class TaskScheduler {
TaskQueue* task_queue() const { return _task_queue.get(); }
+ void set_wg_id(uint64_t wg_id) { this->_wg_id = wg_id; }
+
+ uint64_t get_wg_id() { return _wg_id; }
Review Comment:
warning: method 'get_wg_id' can be made const
[readability-make-member-function-const]
```suggestion
uint64_t get_wg_id() const { return _wg_id; }
```
##########
be/src/runtime/query_context.h:
##########
@@ -151,9 +150,20 @@ class QueryContext {
vectorized::RuntimePredicate& get_runtime_predicate() { return
_runtime_predicate; }
- void set_task_group(taskgroup::TaskGroupPtr& tg) { _task_group = tg; }
+ void set_task_group(taskgroup::TaskGroupPtr& tg) {
+ std::lock_guard<std::shared_mutex> write_lock(_task_group_lock);
+ _task_group = tg;
+ }
- taskgroup::TaskGroup* get_task_group() const { return _task_group.get(); }
+ taskgroup::TaskGroup* get_task_group() {
Review Comment:
warning: method 'get_task_group' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static taskgroup::TaskGroup* get_task_group() {
```
##########
be/src/runtime/query_context.h:
##########
@@ -194,20 +204,33 @@
TUniqueId query_id() const { return _query_id; }
- void set_task_scheduler(pipeline::TaskScheduler* task_scheduler) {
- _task_scheduler = task_scheduler;
+ void set_task_scheduler(std::shared_ptr<pipeline::TaskScheduler>*
task_scheduler) {
+ std::lock_guard<std::shared_mutex> write_lock(_exec_task_sched_mutex);
+ _task_scheduler = *task_scheduler;
}
- pipeline::TaskScheduler* get_task_scheduler() { return _task_scheduler; }
+ pipeline::TaskScheduler* get_task_scheduler() {
+ std::shared_lock<std::shared_mutex> read_lock(_exec_task_sched_mutex);
+ return _task_scheduler.get();
+ }
+
+ pipeline::TaskQueue* get_exec_task_queue();
- void set_scan_task_scheduler(vectorized::SimplifiedScanScheduler*
scan_task_scheduler) {
- _scan_task_scheduler = scan_task_scheduler;
+ void set_scan_task_scheduler(
+ std::shared_ptr<vectorized::SimplifiedScanScheduler>*
scan_task_scheduler) {
+ std::lock_guard<std::shared_mutex> write_lock(_scan_task_sched_mutex);
+ _scan_task_scheduler = *scan_task_scheduler;
}
- vectorized::SimplifiedScanScheduler* get_scan_scheduler() { return
_scan_task_scheduler; }
+ vectorized::SimplifiedScanScheduler* get_scan_scheduler() {
Review Comment:
warning: method 'get_scan_scheduler' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static vectorized::SimplifiedScanScheduler* get_scan_scheduler() {
```
##########
be/src/runtime/query_context.h:
##########
@@ -194,20 +204,33 @@
TUniqueId query_id() const { return _query_id; }
- void set_task_scheduler(pipeline::TaskScheduler* task_scheduler) {
- _task_scheduler = task_scheduler;
+ void set_task_scheduler(std::shared_ptr<pipeline::TaskScheduler>*
task_scheduler) {
+ std::lock_guard<std::shared_mutex> write_lock(_exec_task_sched_mutex);
+ _task_scheduler = *task_scheduler;
}
- pipeline::TaskScheduler* get_task_scheduler() { return _task_scheduler; }
+ pipeline::TaskScheduler* get_task_scheduler() {
+ std::shared_lock<std::shared_mutex> read_lock(_exec_task_sched_mutex);
+ return _task_scheduler.get();
+ }
+
+ pipeline::TaskQueue* get_exec_task_queue();
- void set_scan_task_scheduler(vectorized::SimplifiedScanScheduler*
scan_task_scheduler) {
- _scan_task_scheduler = scan_task_scheduler;
+ void set_scan_task_scheduler(
+ std::shared_ptr<vectorized::SimplifiedScanScheduler>*
scan_task_scheduler) {
+ std::lock_guard<std::shared_mutex> write_lock(_scan_task_sched_mutex);
+ _scan_task_scheduler = *scan_task_scheduler;
}
- vectorized::SimplifiedScanScheduler* get_scan_scheduler() { return
_scan_task_scheduler; }
+ vectorized::SimplifiedScanScheduler* get_scan_scheduler() {
+ std::shared_lock<std::shared_mutex> read_lock(_scan_task_sched_mutex);
+ return _scan_task_scheduler.get();
+ }
pipeline::Dependency* get_execution_dependency() { return
_execution_dependency.get(); }
+ int64_t query_time() { return MonotonicMillis() -
_monotonic_start_time_ms; }
Review Comment:
warning: method 'query_time' can be made const
[readability-make-member-function-const]
```suggestion
int64_t query_time() const { return MonotonicMillis() -
_monotonic_start_time_ms; }
```
##########
be/src/runtime/query_context.h:
##########
@@ -151,9 +150,20 @@
vectorized::RuntimePredicate& get_runtime_predicate() { return
_runtime_predicate; }
- void set_task_group(taskgroup::TaskGroupPtr& tg) { _task_group = tg; }
+ void set_task_group(taskgroup::TaskGroupPtr& tg) {
+ std::lock_guard<std::shared_mutex> write_lock(_task_group_lock);
+ _task_group = tg;
+ }
- taskgroup::TaskGroup* get_task_group() const { return _task_group.get(); }
+ taskgroup::TaskGroup* get_task_group() {
+ std::shared_lock<std::shared_mutex> read_lock(_task_group_lock);
+ return _task_group.get();
+ }
+
+ taskgroup::TaskGroupPtr get_task_group_shared_ptr() {
Review Comment:
warning: method 'get_task_group_shared_ptr' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static taskgroup::TaskGroupPtr get_task_group_shared_ptr() {
```
##########
be/src/runtime/task_group/task_group_manager.cpp:
##########
@@ -237,6 +238,21 @@
LOG(INFO) << "finish clear unused cgroup path";
}
+bool TaskGroupManager::migrate_memory_tracker_to_group(
+ std::shared_ptr<MemTrackerLimiter> mem_tracker, uint64_t src_group_id,
+ uint64_t dst_group_id, std::shared_ptr<taskgroup::TaskGroup>*
dst_group_ptr) {
+ std::lock_guard<std::shared_mutex> write_lock(_group_mutex);
+ if (_task_groups.find(src_group_id) == _task_groups.end() ||
+ _task_groups.find(dst_group_id) == _task_groups.end()) {
+ return false;
Review Comment:
warning: redundant boolean literal in conditional return statement
[readability-simplify-boolean-expr]
be/src/runtime/task_group/task_group_manager.cpp:244:
```diff
- if (_task_groups.find(src_group_id) == _task_groups.end() ||
- _task_groups.find(dst_group_id) == _task_groups.end()) {
- return false;
- }
- _task_groups[src_group_id]->remove_mem_tracker_limiter(mem_tracker);
- *dst_group_ptr = _task_groups[dst_group_id];
- (*dst_group_ptr)->add_mem_tracker_limiter(mem_tracker);
-
- return true;
+ return !_task_groups.find(src_group_id) == _task_groups.end() ||
+ _task_groups.find(dst_group_id) == _task_groups.end();
```
##########
be/src/runtime/query_context.h:
##########
@@ -194,20 +204,33 @@
TUniqueId query_id() const { return _query_id; }
- void set_task_scheduler(pipeline::TaskScheduler* task_scheduler) {
- _task_scheduler = task_scheduler;
+ void set_task_scheduler(std::shared_ptr<pipeline::TaskScheduler>*
task_scheduler) {
+ std::lock_guard<std::shared_mutex> write_lock(_exec_task_sched_mutex);
+ _task_scheduler = *task_scheduler;
}
- pipeline::TaskScheduler* get_task_scheduler() { return _task_scheduler; }
+ pipeline::TaskScheduler* get_task_scheduler() {
Review Comment:
warning: method 'get_task_scheduler' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static pipeline::TaskScheduler* get_task_scheduler() {
```
##########
be/src/runtime/task_group/task_group_manager.cpp:
##########
@@ -237,6 +238,21 @@ void
TaskGroupManager::delete_task_group_by_ids(std::set<uint64_t> used_wg_id) {
LOG(INFO) << "finish clear unused cgroup path";
}
+bool TaskGroupManager::migrate_memory_tracker_to_group(
Review Comment:
warning: method 'migrate_memory_tracker_to_group' can be made static
[readability-convert-member-functions-to-static]
be/src/runtime/task_group/task_group_manager.h:73:
```diff
- bool
migrate_memory_tracker_to_group(std::shared_ptr<MemTrackerLimiter> mem_tracker,
+ static bool
migrate_memory_tracker_to_group(std::shared_ptr<MemTrackerLimiter> mem_tracker,
```
--
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]
