github-actions[bot] commented on code in PR #30014: URL: https://github.com/apache/doris/pull/30014#discussion_r1452951785
########## be/src/service/backend_service.cpp: ########## @@ -93,7 +93,7 @@ struct IngestBinlogArg { TStatus* tstatus; }; -void _ingest_binlog(IngestBinlogArg* arg) { +void _ingest_binlog(StorageEngine& engine, IngestBinlogArg* arg) { Review Comment: warning: function '_ingest_binlog' has cognitive complexity of 69 (threshold 50) [readability-function-cognitive-complexity] ```cpp void _ingest_binlog(StorageEngine& engine, IngestBinlogArg* arg) { ^ ``` <details> <summary>Additional context</summary> **be/src/service/backend_service.cpp:118:** nesting level increased to 1 ```cpp auto set_tstatus = [&tstatus](TStatusCode::type code, std::string error_msg) { ^ ``` **be/src/service/backend_service.cpp:133:** nesting level increased to 1 ```cpp auto get_binlog_info_cb = [&get_binlog_info_url, &binlog_info](HttpClient* client) { ^ ``` **be/src/service/backend_service.cpp:134:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp RETURN_IF_ERROR(client->init(get_binlog_info_url)); ^ ``` **be/src/common/status.h:527:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/service/backend_service.cpp:134:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(client->init(get_binlog_info_url)); ^ ``` **be/src/common/status.h:529:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/service/backend_service.cpp:139:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (!status.ok()) { ^ ``` **be/src/service/backend_service.cpp:157:** nesting level increased to 1 ```cpp auto get_rowset_meta_cb = [&get_rowset_meta_url, &rowset_meta_str](HttpClient* client) { ^ ``` **be/src/service/backend_service.cpp:158:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp RETURN_IF_ERROR(client->init(get_rowset_meta_url)); ^ ``` **be/src/common/status.h:527:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/service/backend_service.cpp:158:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(client->init(get_rowset_meta_url)); ^ ``` **be/src/common/status.h:529:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/service/backend_service.cpp:163:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (!status.ok()) { ^ ``` **be/src/service/backend_service.cpp:170:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (!rowset_meta_pb.ParseFromString(rowset_meta_str)) { ^ ``` **be/src/service/backend_service.cpp:183:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (!rowset_meta->init_from_pb(rowset_meta_pb)) { ^ ``` **be/src/service/backend_service.cpp:200:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp for (int64_t segment_index = 0; segment_index < num_segments; ++segment_index) { ^ ``` **be/src/service/backend_service.cpp:205:** nesting level increased to 2 ```cpp auto get_segment_file_size_cb = [&get_segment_file_size_url, ^ ``` **be/src/service/backend_service.cpp:207:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(client->init(get_segment_file_size_url)); ^ ``` **be/src/common/status.h:527:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/service/backend_service.cpp:207:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(client->init(get_segment_file_size_url)); ^ ``` **be/src/common/status.h:529:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/service/backend_service.cpp:209:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(client->head()); ^ ``` **be/src/common/status.h:527:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/service/backend_service.cpp:209:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(client->head()); ^ ``` **be/src/common/status.h:529:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/service/backend_service.cpp:214:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (!status.ok()) { ^ ``` **be/src/service/backend_service.cpp:227:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (!local_tablet->can_add_binlog(total_size)) { ^ ``` **be/src/service/backend_service.cpp:236:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp for (int64_t segment_index = 0; segment_index < num_segments; ++segment_index) { ^ ``` **be/src/service/backend_service.cpp:242:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (estimate_timeout < config::download_low_speed_time) { ^ ``` **be/src/service/backend_service.cpp:250:** nesting level increased to 2 ```cpp auto get_segment_file_cb = [&get_segment_file_url, &local_segment_path, segment_file_size, ^ ``` **be/src/service/backend_service.cpp:252:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(client->init(get_segment_file_url)); ^ ``` **be/src/common/status.h:527:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/service/backend_service.cpp:252:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(client->init(get_segment_file_url)); ^ ``` **be/src/common/status.h:529:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/service/backend_service.cpp:254:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(client->download(local_segment_path)); ^ ``` **be/src/common/status.h:527:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/service/backend_service.cpp:254:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(client->download(local_segment_path)); ^ ``` **be/src/common/status.h:529:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/service/backend_service.cpp:259:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (ec) { ^ ``` **be/src/service/backend_service.cpp:264:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (local_file_size != segment_file_size) { ^ ``` **be/src/service/backend_service.cpp:276:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (!status.ok()) { ^ ``` **be/src/service/backend_service.cpp:290:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (!status) { ^ ``` **be/src/service/backend_service.cpp:304:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (local_tablet->enable_unique_key_merge_on_write()) { ^ ``` **be/src/service/backend_service.cpp:308:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (!status) { ^ ``` **be/src/service/backend_service.cpp:315:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (segments.size() > 1) { ^ ``` **be/src/service/backend_service.cpp:319:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (!status) { ^ ``` **be/src/service/backend_service.cpp:340:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (!commit_txn_status && !commit_txn_status.is<ErrorCode::PUSH_TRANSACTION_ALREADY_EXIST>()) { ^ ``` **be/src/service/backend_service.cpp:340:** +1 ```cpp if (!commit_txn_status && !commit_txn_status.is<ErrorCode::PUSH_TRANSACTION_ALREADY_EXIST>()) { ^ ``` **be/src/service/backend_service.cpp:351:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (local_tablet->enable_unique_key_merge_on_write()) { ^ ``` </details> ########## be/src/service/backend_service.cpp: ########## @@ -93,7 +93,7 @@ TStatus* tstatus; }; -void _ingest_binlog(IngestBinlogArg* arg) { +void _ingest_binlog(StorageEngine& engine, IngestBinlogArg* arg) { Review Comment: warning: function '_ingest_binlog' exceeds recommended size/complexity thresholds [readability-function-size] ```cpp void _ingest_binlog(StorageEngine& engine, IngestBinlogArg* arg) { ^ ``` <details> <summary>Additional context</summary> **be/src/service/backend_service.cpp:95:** 263 lines including whitespace and comments (threshold 80) ```cpp void _ingest_binlog(StorageEngine& engine, IngestBinlogArg* arg) { ^ ``` </details> ########## be/src/service/internal_service.cpp: ########## @@ -305,7 +311,7 @@ } } -void PInternalServiceImpl::_exec_plan_fragment_in_pthread( +void PInternalService::_exec_plan_fragment_in_pthread( Review Comment: warning: method '_exec_plan_fragment_in_pthread' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::_exec_plan_fragment_in_pthread( ``` ########## be/src/service/internal_service.cpp: ########## @@ -427,7 +434,7 @@ } } -void PInternalServiceImpl::tablet_writer_add_block(google::protobuf::RpcController* controller, +void PInternalService::tablet_writer_add_block(google::protobuf::RpcController* controller, Review Comment: warning: method 'tablet_writer_add_block' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::tablet_writer_add_block(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -601,7 +608,7 @@ } } -void PInternalServiceImpl::fetch_data(google::protobuf::RpcController* controller, +void PInternalService::fetch_data(google::protobuf::RpcController* controller, Review Comment: warning: method 'fetch_data' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::fetch_data(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -1082,7 +1090,7 @@ } } -void PInternalServiceImpl::fetch_cache(google::protobuf::RpcController* controller, +void PInternalService::fetch_cache(google::protobuf::RpcController* controller, Review Comment: warning: method 'fetch_cache' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::fetch_cache(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -561,7 +568,7 @@ } } -void PInternalServiceImpl::cancel_plan_fragment(google::protobuf::RpcController* /*controller*/, +void PInternalService::cancel_plan_fragment(google::protobuf::RpcController* /*controller*/, Review Comment: warning: method 'cancel_plan_fragment' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::cancel_plan_fragment(google::protobuf::RpcController* /*controller*/, ``` ########## be/src/service/internal_service.cpp: ########## @@ -1003,7 +1011,7 @@ st.to_protobuf(response->mutable_status()); } -void PInternalServiceImpl::get_info(google::protobuf::RpcController* controller, +void PInternalService::get_info(google::protobuf::RpcController* controller, Review Comment: warning: method 'get_info' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::get_info(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -327,7 +333,7 @@ st.to_protobuf(response->mutable_status()); } -void PInternalServiceImpl::exec_plan_fragment_prepare(google::protobuf::RpcController* controller, +void PInternalService::exec_plan_fragment_prepare(google::protobuf::RpcController* controller, Review Comment: warning: method 'exec_plan_fragment_prepare' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::exec_plan_fragment_prepare(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -411,7 +418,7 @@ } } -void PInternalServiceImpl::tablet_writer_add_block_by_http( +void PInternalService::tablet_writer_add_block_by_http( Review Comment: warning: method 'tablet_writer_add_block_by_http' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::tablet_writer_add_block_by_http( ``` ########## be/src/service/backend_service.cpp: ########## @@ -548,8 +554,8 @@ } } -void BackendService::submit_routine_load_task(TStatus& t_status, - const std::vector<TRoutineLoadTask>& tasks) { +void BaseBackendService::submit_routine_load_task(TStatus& t_status, Review Comment: warning: method 'submit_routine_load_task' can be made static [readability-convert-member-functions-to-static] ```suggestion static void BaseBackendService::submit_routine_load_task(TStatus& t_status, ``` ########## be/src/service/backend_service.cpp: ########## @@ -366,12 +365,20 @@ using apache::thrift::transport::TTransportException; using apache::thrift::concurrency::ThreadFactory; -BackendService::BackendService(ExecEnv* exec_env) +BaseBackendService::BaseBackendService(ExecEnv* exec_env) : _exec_env(exec_env), _agent_server(new AgentServer(exec_env, *exec_env->master_info())) {} -Status BackendService::create_service(ExecEnv* exec_env, int port, - std::unique_ptr<ThriftServer>* server) { - auto service = std::make_shared<BackendService>(exec_env); +BackendService::BackendService(StorageEngine& engine, ExecEnv* exec_env) + : BaseBackendService(exec_env), _engine(engine) {} + +Status BaseBackendService::create_service(ExecEnv* exec_env, int port, Review Comment: warning: method 'create_service' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status BaseBackendService::create_service(ExecEnv* exec_env, int port, ``` ########## be/src/service/internal_service.cpp: ########## @@ -340,7 +346,7 @@ } } -void PInternalServiceImpl::exec_plan_fragment_start(google::protobuf::RpcController* /*controller*/, +void PInternalService::exec_plan_fragment_start(google::protobuf::RpcController* /*controller*/, Review Comment: warning: method 'exec_plan_fragment_start' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::exec_plan_fragment_start(google::protobuf::RpcController* /*controller*/, ``` ########## be/src/service/backend_service.cpp: ########## @@ -401,29 +408,30 @@ return Status::OK(); } -void BackendService::exec_plan_fragment(TExecPlanFragmentResult& return_val, - const TExecPlanFragmentParams& params) { +void BaseBackendService::exec_plan_fragment(TExecPlanFragmentResult& return_val, + const TExecPlanFragmentParams& params) { LOG(INFO) << "exec_plan_fragment() instance_id=" << print_id(params.params.fragment_instance_id) << " coord=" << params.coord << " backend#=" << params.backend_num; return_val.__set_status(start_plan_fragment_execution(params).to_thrift()); } -Status BackendService::start_plan_fragment_execution(const TExecPlanFragmentParams& exec_params) { +Status BaseBackendService::start_plan_fragment_execution( + const TExecPlanFragmentParams& exec_params) { if (!exec_params.fragment.__isset.output_sink) { return Status::InternalError("missing sink in plan fragment"); } return _exec_env->fragment_mgr()->exec_plan_fragment(exec_params); } -void BackendService::cancel_plan_fragment(TCancelPlanFragmentResult& return_val, - const TCancelPlanFragmentParams& params) { +void BaseBackendService::cancel_plan_fragment(TCancelPlanFragmentResult& return_val, + const TCancelPlanFragmentParams& params) { LOG(INFO) << "cancel_plan_fragment(): instance_id=" << print_id(params.fragment_instance_id); _exec_env->fragment_mgr()->cancel_instance(params.fragment_instance_id, PPlanFragmentCancelReason::INTERNAL_ERROR); } -void BackendService::transmit_data(TTransmitDataResult& return_val, - const TTransmitDataParams& params) { +void BaseBackendService::transmit_data(TTransmitDataResult& return_val, Review Comment: warning: method 'transmit_data' can be made static [readability-convert-member-functions-to-static] be/src/service/backend_service.h:107: ```diff - void transmit_data(TTransmitDataResult& return_val, const TTransmitDataParams& params) override; + static void transmit_data(TTransmitDataResult& return_val, const TTransmitDataParams& params) override; ``` ########## be/src/service/internal_service.cpp: ########## @@ -355,7 +361,7 @@ } } -void PInternalServiceImpl::open_load_stream(google::protobuf::RpcController* controller, +void PInternalService::open_load_stream(google::protobuf::RpcController* controller, Review Comment: warning: method 'open_load_stream' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::open_load_stream(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -253,23 +259,23 @@ PInternalServiceImpl::~PInternalServiceImpl() { CHECK_EQ(0, bthread_key_delete(AsyncIO::btls_io_ctx_key)); } -void PInternalServiceImpl::transmit_data(google::protobuf::RpcController* controller, +void PInternalService::transmit_data(google::protobuf::RpcController* controller, const PTransmitDataParams* request, PTransmitDataResult* response, google::protobuf::Closure* done) {} -void PInternalServiceImpl::transmit_data_by_http(google::protobuf::RpcController* controller, +void PInternalService::transmit_data_by_http(google::protobuf::RpcController* controller, const PEmptyRequest* request, PTransmitDataResult* response, google::protobuf::Closure* done) {} -void PInternalServiceImpl::_transmit_data(google::protobuf::RpcController* controller, +void PInternalService::_transmit_data(google::protobuf::RpcController* controller, const PTransmitDataParams* request, PTransmitDataResult* response, google::protobuf::Closure* done, const Status& extract_st) {} -void PInternalServiceImpl::tablet_writer_open(google::protobuf::RpcController* controller, +void PInternalService::tablet_writer_open(google::protobuf::RpcController* controller, Review Comment: warning: method 'tablet_writer_open' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::tablet_writer_open(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -292,7 +298,7 @@ } } -void PInternalServiceImpl::exec_plan_fragment(google::protobuf::RpcController* controller, +void PInternalService::exec_plan_fragment(google::protobuf::RpcController* controller, Review Comment: warning: method 'exec_plan_fragment' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::exec_plan_fragment(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -457,7 +464,7 @@ } } -void PInternalServiceImpl::tablet_writer_cancel(google::protobuf::RpcController* controller, +void PInternalService::tablet_writer_cancel(google::protobuf::RpcController* controller, Review Comment: warning: method 'tablet_writer_cancel' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::tablet_writer_cancel(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -615,7 +622,7 @@ } } -void PInternalServiceImpl::fetch_table_schema(google::protobuf::RpcController* controller, +void PInternalService::fetch_table_schema(google::protobuf::RpcController* controller, Review Comment: warning: function 'fetch_table_schema' exceeds recommended size/complexity thresholds [readability-function-size] ```cpp void PInternalService::fetch_table_schema(google::protobuf::RpcController* controller, ^ ``` <details> <summary>Additional context</summary> **be/src/service/internal_service.cpp:624:** 103 lines including whitespace and comments (threshold 80) ```cpp void PInternalService::fetch_table_schema(google::protobuf::RpcController* controller, ^ ``` </details> ########## be/src/service/internal_service.cpp: ########## @@ -1108,7 +1116,7 @@ } } -void PInternalServiceImpl::merge_filter(::google::protobuf::RpcController* controller, +void PInternalService::merge_filter(::google::protobuf::RpcController* controller, Review Comment: warning: method 'merge_filter' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::merge_filter(::google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -723,7 +730,7 @@ } } -void PInternalServiceImpl::fetch_arrow_flight_schema(google::protobuf::RpcController* controller, +void PInternalService::fetch_arrow_flight_schema(google::protobuf::RpcController* controller, Review Comment: warning: method 'fetch_arrow_flight_schema' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::fetch_arrow_flight_schema(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -615,7 +622,7 @@ } } -void PInternalServiceImpl::fetch_table_schema(google::protobuf::RpcController* controller, +void PInternalService::fetch_table_schema(google::protobuf::RpcController* controller, Review Comment: warning: method 'fetch_table_schema' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::fetch_table_schema(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -1069,7 +1077,7 @@ } } -void PInternalServiceImpl::update_cache(google::protobuf::RpcController* controller, +void PInternalService::update_cache(google::protobuf::RpcController* controller, Review Comment: warning: method 'update_cache' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::update_cache(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -1095,7 +1103,7 @@ } } -void PInternalServiceImpl::clear_cache(google::protobuf::RpcController* controller, +void PInternalService::clear_cache(google::protobuf::RpcController* controller, Review Comment: warning: method 'clear_cache' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::clear_cache(google::protobuf::RpcController* controller, ``` ########## be/src/service/internal_service.cpp: ########## @@ -1128,7 +1136,7 @@ } } -void PInternalServiceImpl::apply_filter(::google::protobuf::RpcController* controller, +void PInternalService::apply_filter(::google::protobuf::RpcController* controller, Review Comment: warning: method 'apply_filter' can be made static [readability-convert-member-functions-to-static] ```suggestion static void PInternalService::apply_filter(::google::protobuf::RpcController* controller, ``` -- 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