github-actions[bot] commented on code in PR #29153: URL: https://github.com/apache/doris/pull/29153#discussion_r1440099369
########## be/src/exprs/runtime_filter_slots.h: ########## @@ -204,10 +204,10 @@ class VRuntimeFilterSlots { } // publish runtime filter - Status publish() { + Status publish(bool publish_local = false) { Review Comment: warning: method 'publish' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status publish(bool publish_local = false) { ``` ########## be/src/olap/schema_change.cpp: ########## @@ -1035,7 +991,10 @@ Status SchemaChangeHandler::_get_versions_to_be_changed( return Status::OK(); } -Status SchemaChangeHandler::_convert_historical_rowsets(const SchemaChangeParams& sc_params) { +// The `real_alter_version` parameter indicates that the version of [0-real_alter_version] is +// converted from a base tablet, only used for the mow table now. +Status SchemaChangeHandler::_convert_historical_rowsets(const SchemaChangeParams& sc_params, Review Comment: warning: function '_convert_historical_rowsets' exceeds recommended size/complexity thresholds [readability-function-size] ```cpp Status SchemaChangeHandler::_convert_historical_rowsets(const SchemaChangeParams& sc_params, ^ ``` <details> <summary>Additional context</summary> **be/src/olap/schema_change.cpp:995:** 133 lines including whitespace and comments (threshold 80) ```cpp Status SchemaChangeHandler::_convert_historical_rowsets(const SchemaChangeParams& sc_params, ^ ``` </details> ########## be/test/olap/wal_manager_test.cpp: ########## @@ -14,7 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. -#include "olap/wal_manager.h" +#include "olap/wal/wal_manager.h" Review Comment: warning: 'olap/wal/wal_manager.h' file not found [clang-diagnostic-error] ```cpp #include "olap/wal/wal_manager.h" ^ ``` ########## be/src/runtime/group_commit_mgr.cpp: ########## @@ -495,66 +505,24 @@ Status LoadBlockQueue::close_wal() { return Status::OK(); } -bool LoadBlockQueue::has_enough_wal_disk_space( - const std::vector<std::shared_ptr<vectorized::Block>>& blocks, const TUniqueId& load_id, - bool is_blocks_contain_all_load_data) { - size_t blocks_size = 0; - for (auto block : blocks) { - blocks_size += block->bytes(); - } - size_t content_length = 0; - Status st = ExecEnv::GetInstance()->group_commit_mgr()->get_load_info(load_id, &content_length); - if (st.ok()) { - RETURN_IF_ERROR(ExecEnv::GetInstance()->group_commit_mgr()->remove_load_info(load_id)); - } else { - return Status::InternalError("can not find load id."); - } - size_t pre_allocated = is_blocks_contain_all_load_data - ? blocks_size - : (blocks_size > content_length ? blocks_size : content_length); +bool LoadBlockQueue::has_enough_wal_disk_space(size_t pre_allocated) { Review Comment: warning: method 'has_enough_wal_disk_space' can be made static [readability-convert-member-functions-to-static] be/src/runtime/group_commit_mgr.h:72: ```diff - bool has_enough_wal_disk_space(size_t pre_allocated); + static bool has_enough_wal_disk_space(size_t pre_allocated); ``` ########## be/src/runtime/group_commit_mgr.h: ########## @@ -45,58 +45,73 @@ class LoadBlockQueue { LoadBlockQueue(const UniqueId& load_instance_id, std::string& label, int64_t txn_id, int64_t schema_version, std::shared_ptr<std::atomic_size_t> all_block_queues_bytes, - bool wait_internal_group_commit_finish, int64_t group_commit_interval_ms) + bool wait_internal_group_commit_finish, int64_t group_commit_interval_ms, + int64_t group_commit_data_bytes) : load_instance_id(load_instance_id), label(label), txn_id(txn_id), schema_version(schema_version), wait_internal_group_commit_finish(wait_internal_group_commit_finish), + _group_commit_interval_ms(group_commit_interval_ms), _start_time(std::chrono::steady_clock::now()), - _all_block_queues_bytes(all_block_queues_bytes), - _group_commit_interval_ms(group_commit_interval_ms) {}; + _group_commit_data_bytes(group_commit_data_bytes), + _all_block_queues_bytes(all_block_queues_bytes) {}; - Status add_block(std::shared_ptr<vectorized::Block> block, bool write_wal); + Status add_block(RuntimeState* runtime_state, std::shared_ptr<vectorized::Block> block, + bool write_wal); Status get_block(RuntimeState* runtime_state, vectorized::Block* block, bool* find_block, bool* eos); Status add_load_id(const UniqueId& load_id); void remove_load_id(const UniqueId& load_id); void cancel(const Status& st); + bool need_commit() { return _need_commit; } Review Comment: warning: method 'need_commit' can be made const [readability-make-member-function-const] ```suggestion bool need_commit() const { return _need_commit; } ``` ########## be/src/vec/olap/block_reader.cpp: ########## @@ -429,12 +430,16 @@ Status BlockReader::_unique_key_next_block(Block* block, bool* eof) { return Status::OK(); } -void BlockReader::_insert_data_normal(MutableColumns& columns) { +Status BlockReader::_insert_data_normal(MutableColumns& columns) { Review Comment: warning: method '_insert_data_normal' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status BlockReader::_insert_data_normal(MutableColumns& columns) { ``` -- 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