github-actions[bot] commented on code in PR #29560:
URL: https://github.com/apache/doris/pull/29560#discussion_r1441994238
##########
be/src/olap/wal/wal_manager.cpp:
##########
@@ -160,76 +161,58 @@
&_update_wal_dirs_info_thread);
}
-void WalManager::add_wal_status_queue(int64_t table_id, int64_t wal_id,
WalStatus wal_status) {
- std::lock_guard<std::shared_mutex> wrlock(_wal_status_lock);
+void WalManager::add_wal_queue(int64_t table_id, int64_t wal_id) {
+ std::lock_guard<std::shared_mutex> wrlock(_wal_queue_lock);
LOG(INFO) << "add wal queue "
- << ",table_id:" << table_id << ",wal_id:" << wal_id <<
",status:" << wal_status;
- auto it = _wal_status_queues.find(table_id);
- if (it == _wal_status_queues.end()) {
- std::unordered_map<int64_t, WalStatus> tmp_map;
- tmp_map.emplace(wal_id, wal_status);
- _wal_status_queues.emplace(table_id, tmp_map);
+ << ",table_id:" << table_id << ",wal_id:" << wal_id;
+ auto it = _wal_queues.find(table_id);
+ if (it == _wal_queues.end()) {
+ std::set<int64_t> tmp_set;
+ tmp_set.insert(wal_id);
+ _wal_queues.emplace(table_id, tmp_set);
} else {
- it->second.emplace(wal_id, wal_status);
+ it->second.insert(wal_id);
}
}
-Status WalManager::erase_wal_status_queue(int64_t table_id, int64_t wal_id) {
- std::lock_guard<std::shared_mutex> wrlock(_wal_status_lock);
- auto it = _wal_status_queues.find(table_id);
+void WalManager::erase_wal_queue(int64_t table_id, int64_t wal_id) {
+ std::lock_guard<std::shared_mutex> wrlock(_wal_queue_lock);
+ auto it = _wal_queues.find(table_id);
LOG(INFO) << "remove wal queue "
<< ",table_id:" << table_id << ",wal_id:" << wal_id;
- if (it == _wal_status_queues.end()) {
- return Status::InternalError("table_id " + std::to_string(table_id) +
- " not found in wal status queue");
- } else {
+ if (it != _wal_queues.end()) {
it->second.erase(wal_id);
if (it->second.empty()) {
- _wal_status_queues.erase(table_id);
+ _wal_queues.erase(table_id);
}
}
- return Status::OK();
}
-Status WalManager::get_wal_status_queue_size(const PGetWalQueueSizeRequest*
request,
- PGetWalQueueSizeResponse*
response) {
- std::lock_guard<std::shared_mutex> wrlock(_wal_status_lock);
+size_t WalManager::_get_wal_queue_size(int64_t table_id) {
Review Comment:
warning: method '_get_wal_queue_size' can be made static
[readability-convert-member-functions-to-static]
be/src/olap/wal/wal_manager.h:90:
```diff
- size_t _get_wal_queue_size(int64_t table_id);
+ static size_t _get_wal_queue_size(int64_t table_id);
```
##########
be/src/olap/wal/wal_manager.cpp:
##########
@@ -160,76 +161,58 @@
&_update_wal_dirs_info_thread);
}
-void WalManager::add_wal_status_queue(int64_t table_id, int64_t wal_id,
WalStatus wal_status) {
- std::lock_guard<std::shared_mutex> wrlock(_wal_status_lock);
+void WalManager::add_wal_queue(int64_t table_id, int64_t wal_id) {
+ std::lock_guard<std::shared_mutex> wrlock(_wal_queue_lock);
LOG(INFO) << "add wal queue "
- << ",table_id:" << table_id << ",wal_id:" << wal_id <<
",status:" << wal_status;
- auto it = _wal_status_queues.find(table_id);
- if (it == _wal_status_queues.end()) {
- std::unordered_map<int64_t, WalStatus> tmp_map;
- tmp_map.emplace(wal_id, wal_status);
- _wal_status_queues.emplace(table_id, tmp_map);
+ << ",table_id:" << table_id << ",wal_id:" << wal_id;
+ auto it = _wal_queues.find(table_id);
+ if (it == _wal_queues.end()) {
+ std::set<int64_t> tmp_set;
+ tmp_set.insert(wal_id);
+ _wal_queues.emplace(table_id, tmp_set);
} else {
- it->second.emplace(wal_id, wal_status);
+ it->second.insert(wal_id);
}
}
-Status WalManager::erase_wal_status_queue(int64_t table_id, int64_t wal_id) {
- std::lock_guard<std::shared_mutex> wrlock(_wal_status_lock);
- auto it = _wal_status_queues.find(table_id);
+void WalManager::erase_wal_queue(int64_t table_id, int64_t wal_id) {
Review Comment:
warning: method 'erase_wal_queue' can be made static
[readability-convert-member-functions-to-static]
be/src/olap/wal/wal_manager.h:72:
```diff
- void erase_wal_queue(int64_t table_id, int64_t wal_id);
+ static void erase_wal_queue(int64_t table_id, int64_t wal_id);
```
##########
be/src/olap/wal/wal_table.cpp:
##########
@@ -339,10 +299,11 @@
size_t WalTable::size() {
std::lock_guard<std::mutex> lock(_replay_wal_lock);
- return _replay_wal_map.size();
+ return _replay_wal_map.size() + _replaying_queue.size();
}
-Status WalTable::_get_column_info(int64_t db_id, int64_t tb_id) {
+Status WalTable::_get_column_info(int64_t db_id, int64_t tb_id,
Review Comment:
warning: method '_get_column_info' can be made static
[readability-convert-member-functions-to-static]
be/src/olap/wal/wal_table.h:50:
```diff
- Status _get_column_info(int64_t db_id, int64_t tb_id,
+ static Status _get_column_info(int64_t db_id, int64_t tb_id,
```
##########
be/src/olap/wal/wal_manager.cpp:
##########
@@ -160,76 +161,58 @@ Status WalManager::_init_wal_dirs_info() {
&_update_wal_dirs_info_thread);
}
-void WalManager::add_wal_status_queue(int64_t table_id, int64_t wal_id,
WalStatus wal_status) {
- std::lock_guard<std::shared_mutex> wrlock(_wal_status_lock);
+void WalManager::add_wal_queue(int64_t table_id, int64_t wal_id) {
Review Comment:
warning: method 'add_wal_queue' can be made static
[readability-convert-member-functions-to-static]
be/src/olap/wal/wal_manager.h:71:
```diff
- void add_wal_queue(int64_t table_id, int64_t wal_id);
+ static void add_wal_queue(int64_t table_id, int64_t wal_id);
```
##########
be/src/vec/sink/writer/vwal_writer.h:
##########
@@ -37,6 +37,9 @@ class VWalWriter {
Status write_wal(vectorized::Block* block);
Status close();
+private:
+ Status _create_wal_writer(int64_t wal_id, std::shared_ptr<WalWriter>&
wal_writer);
+
private:
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
<details>
<summary>Additional context</summary>
**be/src/vec/sink/writer/vwal_writer.h:39:** previously declared here
```cpp
private:
^
```
</details>
##########
be/src/olap/wal/wal_manager.h:
##########
@@ -72,25 +66,15 @@ class WalManager {
Status create_wal_path(int64_t db_id, int64_t table_id, int64_t wal_id,
const std::string& label, std::string& base_path);
Status get_wal_path(int64_t wal_id, std::string& wal_path);
- Status delete_wal(int64_t wal_id, size_t block_queue_pre_allocated = 0);
+ Status delete_wal(int64_t table_id, int64_t wal_id, size_t
block_queue_pre_allocated = 0);
+ Status rename_to_tmp_path(const std::string wal, int64_t table_id, int64_t
wal_id);
Review Comment:
warning: parameter 'wal' is const-qualified in the function declaration;
const-qualification of parameters only has an effect in function definitions
[readability-avoid-const-params-in-decls]
```suggestion
Status rename_to_tmp_path(std::string wal, int64_t table_id, int64_t
wal_id);
```
##########
be/src/olap/wal/wal_table.cpp:
##########
@@ -205,76 +179,66 @@ Status WalTable::_try_abort_txn(int64_t db_id, int64_t
wal_id) {
Status WalTable::_replay_wal_internal(const std::string& wal) {
LOG(INFO) << "Start replay wal for db=" << _db_id << ", table=" <<
_table_id << ", wal=" << wal;
- std::shared_ptr<std::pair<int64_t, std::string>> pair = nullptr;
- RETURN_IF_ERROR(_parse_wal_path(wal, pair));
- auto wal_id = pair->first;
- auto label = pair->second;
+ int64_t wal_id = 0;
+ std::string label = "";
+ RETURN_IF_ERROR(_parse_wal_path(wal, wal_id, label));
#ifndef BE_TEST
auto st = _try_abort_txn(_db_id, wal_id);
if (!st.ok()) {
LOG(WARNING) << "abort txn " << wal_id << " fail";
}
- RETURN_IF_ERROR(_get_column_info(_db_id, _table_id));
#endif
RETURN_IF_ERROR(_replay_one_txn_with_stremaload(wal_id, wal, label));
return Status::OK();
}
-Status WalTable::_parse_wal_path(const std::string& wal,
- std::shared_ptr<std::pair<int64_t,
std::string>>& pair) {
- std::vector<std::string> path_element;
- doris::vectorized::WalReader::string_split(wal, "/", path_element);
- auto pos = path_element[path_element.size() - 1].find("_");
+Status WalTable::_parse_wal_path(const std::string& wal, int64_t& wal_id,
std::string& label) {
Review Comment:
warning: method '_parse_wal_path' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status WalTable::_parse_wal_path(const std::string& wal, int64_t&
wal_id, std::string& label) {
```
--
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
