github-actions[bot] commented on code in PR #26198:
URL: https://github.com/apache/doris/pull/26198#discussion_r1384296237
##########
be/src/runtime/load_stream.h:
##########
@@ -130,16 +127,31 @@ class LoadStream : public brpc::StreamInputHandler {
void _parse_header(butil::IOBuf* const message, PStreamHeader& hdr);
void _dispatch(StreamId id, const PStreamHeader& hdr, butil::IOBuf* data);
Status _append_data(const PStreamHeader& header, butil::IOBuf* data);
- void _report_result(StreamId stream, Status& st, std::vector<int64_t>*
success_tablet_ids,
- std::vector<int64_t>* failed_tablet_ids);
+
+ void _report_result(StreamId stream, const Status& st,
+ const std::vector<int64_t>& success_tablet_ids,
+ const std::vector<int64_t>& failed_tablet_ids);
+
+ // report failure for one message
+ void _report_failure(StreamId stream, const Status& status, const
PStreamHeader& header) {
Review Comment:
warning: method '_report_failure' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static void _report_failure(StreamId stream, const Status& status, const
PStreamHeader& header) {
```
##########
be/test/runtime/load_stream_test.cpp:
##########
@@ -491,7 +493,7 @@ class LoadStreamMgrTest : public testing::Test {
: _heavy_work_pool(4, 32, "load_stream_test_heavy"),
_light_work_pool(4, 32, "load_stream_test_light") {}
- void close_load(MockSinkClient& client, uint32_t sender_id) {
+ void close_load(MockSinkClient& client, uint32_t sender_id =
NORMAL_SENDER_ID) {
Review Comment:
warning: method 'close_load' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static void close_load(MockSinkClient& client, uint32_t sender_id =
NORMAL_SENDER_ID) {
```
##########
be/test/runtime/load_stream_test.cpp:
##########
@@ -657,25 +664,52 @@
// <client, index, bucket>
// one client
-TEST_F(LoadStreamMgrTest, one_client_abnormal_load) {
+TEST_F(LoadStreamMgrTest, one_client_normal) {
MockSinkClient client;
auto st = client.connect_stream();
EXPECT_TRUE(st.ok());
- write_abnormal_load(client);
- // TODO check abnormal load id
+ write_normal(client);
reset_response_stat();
- close_load(client, 1);
+ close_load(client, ABNORMAL_SENDER_ID);
wait_for_ack(1);
EXPECT_EQ(g_response_stat.num, 1);
EXPECT_EQ(g_response_stat.success_tablet_ids.size(), 0);
+ EXPECT_EQ(g_response_stat.failed_tablet_ids.size(), 0);
EXPECT_EQ(_load_stream_mgr->get_load_stream_num(), 1);
- close_load(client, 0);
+ close_load(client);
wait_for_ack(2);
EXPECT_EQ(g_response_stat.num, 2);
EXPECT_EQ(g_response_stat.success_tablet_ids.size(), 1);
+ EXPECT_EQ(g_response_stat.failed_tablet_ids.size(), 0);
+ EXPECT_EQ(g_response_stat.success_tablet_ids[0], NORMAL_TABLET_ID);
+
+ // server will close stream on CLOSE_LOAD
+ wait_for_close();
+ EXPECT_EQ(_load_stream_mgr->get_load_stream_num(), 0);
+}
+
+TEST_F(LoadStreamMgrTest, one_client_abnormal_load) {
Review Comment:
warning: all parameters should be named in a function
[readability-named-parameter]
```suggestion
TEST_F(LoadStreamMgrTest /*unused*/, one_client_abnormal_load /*unused*/) {
```
##########
be/test/runtime/load_stream_test.cpp:
##########
@@ -1132,4 +1166,77 @@
EXPECT_EQ(_load_stream_mgr->get_load_stream_num(), 0);
}
+TEST_F(LoadStreamMgrTest, two_client_one_close_before_the_other_open) {
Review Comment:
warning: all parameters should be named in a function
[readability-named-parameter]
```suggestion
TEST_F(LoadStreamMgrTest /*unused*/,
two_client_one_close_before_the_other_open /*unused*/) {
```
##########
be/test/runtime/load_stream_test.cpp:
##########
@@ -657,25 +664,52 @@
// <client, index, bucket>
// one client
-TEST_F(LoadStreamMgrTest, one_client_abnormal_load) {
+TEST_F(LoadStreamMgrTest, one_client_normal) {
Review Comment:
warning: all parameters should be named in a function
[readability-named-parameter]
```suggestion
TEST_F(LoadStreamMgrTest /*unused*/, one_client_normal /*unused*/) {
```
##########
be/test/runtime/load_stream_test.cpp:
##########
@@ -1132,4 +1166,77 @@
EXPECT_EQ(_load_stream_mgr->get_load_stream_num(), 0);
}
+TEST_F(LoadStreamMgrTest, two_client_one_close_before_the_other_open) {
+ MockSinkClient clients[2];
+
+ EXPECT_TRUE(clients[0].connect_stream(NORMAL_SENDER_ID, 2).ok());
+
+ reset_response_stat();
+
+ std::vector<std::string> segment_data;
+ segment_data.resize(6);
Review Comment:
warning: 6 is a magic number; consider replacing it with a named constant
[readability-magic-numbers]
```cpp
segment_data.resize(6);
^
```
--
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
