github-actions[bot] commented on code in PR #27726:
URL: https://github.com/apache/doris/pull/27726#discussion_r1430949149
##########
be/src/olap/wal_manager.cpp:
##########
@@ -410,4 +474,36 @@
return Status::OK();
}
+bool WalManager::is_wal_disk_space_enough() {
Review Comment:
warning: method 'is_wal_disk_space_enough' can be made static
[readability-convert-member-functions-to-static]
be/src/olap/wal_manager.h:84:
```diff
- bool is_wal_disk_space_enough();
+ static bool is_wal_disk_space_enough();
```
##########
be/src/olap/wal_manager.cpp:
##########
@@ -410,4 +474,36 @@
return Status::OK();
}
+bool WalManager::is_wal_disk_space_enough() {
+ // if all disks space usage < 80%
+ std::shared_lock l(wal_back_pressure_lock);
+ for (const auto& pair : _wal_dir_to_disk_usage_map) {
+ if (_wal_dir_to_max_limit_map.contains(pair.first) &&
+ *pair.second >= _wal_dir_to_max_limit_map[pair.first] * 0.8) {
+ return false;
+ }
+ }
+ return true;
+}
+
+const string& WalManager::get_min_disk_usage_wal_dir() {
+ std::shared_lock<std::shared_mutex> l(wal_back_pressure_lock);
+ return _wal_dirs.size() == 1
+ ? _wal_dirs[0]
+ : *std::min_element(_wal_dirs.begin(), _wal_dirs.end(),
+ [this](const std::string& dir1, const
std::string& dir2) {
+ return
_wal_dir_to_disk_usage_map[dir1]->load() <
+
_wal_dir_to_disk_usage_map[dir2]->load();
+ });
+}
+
+const string& WalManager::get_random_wal_dir() {
Review Comment:
warning: method 'get_random_wal_dir' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static const string& WalManager::get_random_wal_dir() {
```
##########
be/src/runtime/group_commit_mgr.cpp:
##########
@@ -257,19 +261,20 @@ Status GroupCommitTable::_create_group_commit_load(
params.desc_tbl.slotDescriptors,
be_exe_version));
_cv.notify_all();
}
- st = _exec_plan_fragment(_db_id, _table_id, label, txn_id, is_pipeline,
params,
- pipeline_params);
+ st = _exec_plan_fragment(_db_id, _table_id, label, txn_id, is_pipeline,
params, pipeline_params,
+ write_wal);
if (!st.ok()) {
static_cast<void>(_finish_group_commit_load(_db_id, _table_id, label,
txn_id, instance_id,
- st, true, nullptr));
+ st, true, nullptr,
write_wal));
}
return st;
}
Status GroupCommitTable::_finish_group_commit_load(int64_t db_id, int64_t
table_id,
Review Comment:
warning: function '_finish_group_commit_load' has cognitive complexity of 74
(threshold 50) [readability-function-cognitive-complexity]
```cpp
Status GroupCommitTable::_finish_group_commit_load(int64_t db_id, int64_t
table_id,
^
```
<details>
<summary>Additional context</summary>
**be/src/runtime/group_commit_mgr.cpp:279:** +1, including nesting penalty
of 0, nesting level increased to 1
```cpp
if (status.ok()) {
^
```
**be/src/runtime/group_commit_mgr.cpp:286:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
if (state) {
^
```
**be/src/runtime/group_commit_mgr.cpp:293:** nesting level increased to 2
```cpp
[&request, &result](FrontendServiceConnection& client) {
^
```
**be/src/runtime/group_commit_mgr.cpp:298:** +1, nesting level increased to 1
```cpp
} else {
^
```
**be/src/runtime/group_commit_mgr.cpp:309:** nesting level increased to 2
```cpp
[&request, &result](FrontendServiceConnection& client) {
^
```
**be/src/runtime/group_commit_mgr.cpp:319:** +1, including nesting penalty
of 0, nesting level increased to 1
```cpp
if (it != _load_block_queues.end()) {
^
```
**be/src/runtime/group_commit_mgr.cpp:322:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(load_block_queue->close_wal());
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:322:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(load_block_queue->close_wal());
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:323:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
if (prepare_failed || !status.ok()) {
^
```
**be/src/runtime/group_commit_mgr.cpp:323:** +1
```cpp
if (prepare_failed || !status.ok()) {
^
```
**be/src/runtime/group_commit_mgr.cpp:334:** +1, including nesting penalty
of 0, nesting level increased to 1
```cpp
if (!st.ok()) {
^
```
**be/src/runtime/group_commit_mgr.cpp:340:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
if (!prepare_failed) {
^
```
**be/src/runtime/group_commit_mgr.cpp:342:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->get_wal_path(txn_id,
wal_path));
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:342:** +4, including nesting penalty
of 3, nesting level increased to 4
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->get_wal_path(txn_id,
wal_path));
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:343:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->add_recover_wal(
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:343:** +4, including nesting penalty
of 3, nesting level increased to 4
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->add_recover_wal(
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:348:** +1, nesting level increased to 2
```cpp
} else {
^
```
**be/src/runtime/group_commit_mgr.cpp:349:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->delete_wal(txn_id));
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:349:** +4, including nesting penalty
of 3, nesting level increased to 4
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->delete_wal(txn_id));
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:350:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->erase_wal_status_queue(table_id, txn_id));
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:350:** +4, including nesting penalty
of 3, nesting level increased to 4
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->erase_wal_status_queue(table_id, txn_id));
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:355:** +1, including nesting penalty
of 0, nesting level increased to 1
```cpp
if (!prepare_failed && !result_status.ok() &&
^
```
**be/src/runtime/group_commit_mgr.cpp:355:** +1
```cpp
if (!prepare_failed && !result_status.ok() &&
^
```
**be/src/runtime/group_commit_mgr.cpp:358:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->get_wal_path(txn_id,
wal_path));
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:358:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->get_wal_path(txn_id,
wal_path));
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:359:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->add_recover_wal(std::to_string(db_id),
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:359:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->add_recover_wal(std::to_string(db_id),
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:364:** +1, nesting level increased to 1
```cpp
} else {
^
```
**be/src/runtime/group_commit_mgr.cpp:365:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->delete_wal(txn_id));
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:365:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->delete_wal(txn_id));
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:366:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
if (write_wal) {
^
```
**be/src/runtime/group_commit_mgr.cpp:371:** +2, including nesting penalty
of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->erase_wal_status_queue(table_id, txn_id));
^
```
**be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/runtime/group_commit_mgr.cpp:371:** +3, including nesting penalty
of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_exec_env->wal_mgr()->erase_wal_status_queue(table_id, txn_id));
^
```
**be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/runtime/group_commit_mgr.cpp:376:** +1, including nesting penalty
of 0, nesting level increased to 1
```cpp
if (prepare_failed) {
^
```
**be/src/runtime/group_commit_mgr.cpp:378:** +1, nesting level increased to 1
```cpp
} else {
^
```
**be/src/runtime/group_commit_mgr.cpp:382:** +1, including nesting penalty
of 0, nesting level increased to 1
```cpp
if (state && !(state->get_error_log_file_path().empty())) {
^
```
**be/src/runtime/group_commit_mgr.cpp:382:** +1
```cpp
if (state && !(state->get_error_log_file_path().empty())) {
^
```
</details>
##########
be/src/olap/wal_manager.cpp:
##########
@@ -80,11 +87,72 @@ Status WalManager::init() {
}
RETURN_IF_ERROR(scan_wals(wal_dir));
}
+ RETURN_IF_ERROR(init_wal_limit());
return Thread::create(
"WalMgr", "replay_wal", [this]() {
static_cast<void>(this->replay()); },
&_replay_thread);
}
+Status WalManager::init_wal_dirs() {
Review Comment:
warning: method 'init_wal_dirs' can be made static
[readability-convert-member-functions-to-static]
be/src/olap/wal_manager.h:79:
```diff
- Status init_wal_dirs();
+ static Status init_wal_dirs();
```
##########
be/src/runtime/group_commit_mgr.h:
##########
@@ -46,7 +52,15 @@ class LoadBlockQueue {
_start_time(std::chrono::steady_clock::now()),
_all_block_queues_bytes(all_block_queues_bytes),
_group_commit_interval_ms(group_commit_interval_ms) {
- _single_block_queue_bytes = std::make_shared<std::atomic_size_t>(0);
+ single_block_queue_bytes = std::make_shared<std::atomic_size_t>(0);
+ if (write_wal) {
+ auto wal_mgr = ExecEnv::GetInstance()->wal_mgr();
Review Comment:
warning: 'auto wal_mgr' can be declared as 'auto *wal_mgr'
[readability-qualified-auto]
```suggestion
auto *wal_mgr = ExecEnv::GetInstance()->wal_mgr();
```
##########
be/src/olap/wal_manager.cpp:
##########
@@ -410,4 +474,36 @@
return Status::OK();
}
+bool WalManager::is_wal_disk_space_enough() {
+ // if all disks space usage < 80%
+ std::shared_lock l(wal_back_pressure_lock);
+ for (const auto& pair : _wal_dir_to_disk_usage_map) {
+ if (_wal_dir_to_max_limit_map.contains(pair.first) &&
+ *pair.second >= _wal_dir_to_max_limit_map[pair.first] * 0.8) {
+ return false;
+ }
+ }
+ return true;
+}
+
+const string& WalManager::get_min_disk_usage_wal_dir() {
Review Comment:
warning: method 'get_min_disk_usage_wal_dir' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static const string& WalManager::get_min_disk_usage_wal_dir() {
```
##########
be/src/runtime/group_commit_mgr.cpp:
##########
@@ -472,4 +486,22 @@
}
return Status::OK();
}
+
+Status LoadBlockQueue::wal_back_pressure() {
Review Comment:
warning: method 'wal_back_pressure' can be made static
[readability-convert-member-functions-to-static]
be/src/runtime/group_commit_mgr.h:74:
```diff
- Status wal_back_pressure();
+ static Status wal_back_pressure();
```
--
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
