From 13403598f641b06705a396ee6d02f53d449bb55d Mon Sep 17 00:00:00 2001 From: Jie Yao Date: Fri, 15 Nov 2024 08:24:18 -0700 Subject: [PATCH 1/3] fix get peer info --- conanfile.py | 2 +- src/lib/repl_service_ctx.cpp | 36 ++++++++++++++++++++++-------------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/conanfile.py b/conanfile.py index 4a8bebb..41b4092 100644 --- a/conanfile.py +++ b/conanfile.py @@ -10,7 +10,7 @@ class NuRaftMesgConan(ConanFile): name = "nuraft_mesg" - version = "3.6.2" + version = "3.6.3" homepage = "https://github.com/eBay/nuraft_mesg" description = "A gRPC service for NuRAFT" diff --git a/src/lib/repl_service_ctx.cpp b/src/lib/repl_service_ctx.cpp index fd2046e..2e1c18e 100644 --- a/src/lib/repl_service_ctx.cpp +++ b/src/lib/repl_service_ctx.cpp @@ -89,24 +89,32 @@ const std::string& repl_service_ctx::raft_leader_id() const { std::vector< peer_info > repl_service_ctx::get_raft_status() const { std::vector< peer_info > peers; - if (!is_raft_leader()) return peers; if (!_server) return peers; - auto pinfo_all = _server->get_peer_info_all(); - // add leader to the list - nuraft::raft_server::peer_info pi_leader; - pi_leader.id_ = _server->get_id(); - pi_leader.last_log_idx_ = _server->get_last_log_idx(); - pinfo_all.emplace_back(pi_leader); - - for (auto const& pinfo : pinfo_all) { - std::string_view peer_id; - if (auto srv_config = _server->get_srv_config(pinfo.id_); nullptr != srv_config) { - peer_id = srv_config->get_endpoint(); + if (is_raft_leader()) { + // leader can get all the peer info + auto pinfo_all = _server->get_peer_info_all(); + // get all the peer info except the leader + for (auto const& pinfo : pinfo_all) { + std::string_view peer_id; + if (auto srv_config = _server->get_srv_config(pinfo.id_); nullptr != srv_config) { + peer_id = srv_config->get_endpoint(); + } + if (peer_id.empty()) { + // if remove_member case , leader will first remove the member from peer_list, and then apply the new + // cluster configuraiton which excludes the removed member, so there is case that the peer_id is not in + // the config of leader. + LOGW("do not find peer id {} in the conifg of leader", pinfo.id_); + continue; + } + peers.emplace_back(peer_info{std::string(peer_id), pinfo.last_log_idx_, pinfo.last_succ_resp_us_}); } - DEBUG_ASSERT(!peer_id.empty(), "Unknown peer in config"); - peers.emplace_back(peer_info{std::string(peer_id), pinfo.last_log_idx_, pinfo.last_succ_resp_us_}); } + + // add the peer info of itself(leader or follower) , which is useful for upper layer + // from the view a node itself, last_succ_resp_us_ make no sense, so set it to 0 + peers.emplace_back(peer_info{std::string(_server->get_id()), _server->get_last_log_idx(), 0}); + return peers; } From 4e148dcbb6be882ed089b75b1aa7aab3c2554656 Mon Sep 17 00:00:00 2001 From: Jie Yao Date: Fri, 15 Nov 2024 10:00:19 -0700 Subject: [PATCH 2/3] fix build --- src/lib/repl_service_ctx.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/repl_service_ctx.cpp b/src/lib/repl_service_ctx.cpp index 2e1c18e..e8dbc42 100644 --- a/src/lib/repl_service_ctx.cpp +++ b/src/lib/repl_service_ctx.cpp @@ -111,9 +111,12 @@ std::vector< peer_info > repl_service_ctx::get_raft_status() const { } } + auto my_id = _server->get_id(); + auto my_peer_id = _server->get_srv_config(my_id)->get_endpoint(); + // add the peer info of itself(leader or follower) , which is useful for upper layer // from the view a node itself, last_succ_resp_us_ make no sense, so set it to 0 - peers.emplace_back(peer_info{std::string(_server->get_id()), _server->get_last_log_idx(), 0}); + peers.emplace_back(my_peer_id, _server->get_last_log_idx(), 0); return peers; } From d43f11b67bab4a94b228c5e229cd62d4c83c7127 Mon Sep 17 00:00:00 2001 From: Jie Yao Date: Fri, 15 Nov 2024 20:10:00 -0700 Subject: [PATCH 3/3] fix UT --- src/tests/data_service_tests.cpp | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/tests/data_service_tests.cpp b/src/tests/data_service_tests.cpp index ba8136e..e361ffd 100644 --- a/src/tests/data_service_tests.cpp +++ b/src/tests/data_service_tests.cpp @@ -94,7 +94,6 @@ TEST_F(DataServiceFixture, BasicTest1) { return folly::Unit(); })); - auto repl_ctx1 = sm1->get_repl_context(); for (auto svr : repl_ctx1->_server->get_config()->get_servers()) { if (svr->get_endpoint() == to_string(app_1_->id_)) continue; @@ -113,25 +112,26 @@ TEST_F(DataServiceFixture, BasicTest1) { io_blob_list_t big_cli_buf; test_state_mgr::fill_data_vec_big(big_cli_buf, 4 * 1024 * 1024); sm1->data_service_request_unidirectional(nuraft_mesg::role_regex::ALL, SEND_DATA, big_cli_buf) - .deferValue([](auto e) -> NullResult { - EXPECT_TRUE(e.hasValue()); - return folly::Unit(); - }).get(); + .deferValue([](auto e) -> NullResult { + EXPECT_TRUE(e.hasValue()); + return folly::Unit(); + }) + .get(); LOGINFO("End large object write test") LOGINFO("Starting large object read test") sm4_1->data_service_request_bidirectional(nuraft_mesg::role_regex::LEADER, REQUEST_DATA, big_cli_buf) - .deferValue([](auto e) -> NullResult { - EXPECT_TRUE(e.hasValue()); - test_state_mgr::verify_data(e.value().response_blob()); - return folly::Unit(); - }).get(); + .deferValue([](auto e) -> NullResult { + EXPECT_TRUE(e.hasValue()); + test_state_mgr::verify_data(e.value().response_blob()); + return folly::Unit(); + }) + .get(); LOGINFO("End large object read test") for (auto& buf : big_cli_buf) { buf.buf_free(); } - // add a new member to data_service_test_group and check if repl_ctx4 sends data to newly added member auto add_3 = app_4->instance_->add_member(data_group, app_3_->id_); std::this_thread::sleep_for(std::chrono::seconds(1)); @@ -180,12 +180,13 @@ TEST_F(DataServiceFixture, BasicTest2) { auto repl_ctx_2 = app_2_->state_mgr_map_[group_id_]->get_repl_context(); EXPECT_TRUE(repl_ctx_2 && !repl_ctx_2->is_raft_leader()); EXPECT_TRUE(repl_ctx_2 && repl_ctx_2->raft_leader_id() == to_string(app_1_->id_)); - EXPECT_TRUE(repl_ctx && repl_ctx_2->get_raft_status().size() == 0); + // if it`s a follower, it should have only one peer info of itself + EXPECT_TRUE(repl_ctx && repl_ctx_2->get_raft_status().size() == 1); auto repl_ctx_3 = app_3_->state_mgr_map_[group_id_]->get_repl_context(); EXPECT_TRUE(repl_ctx_3 && !repl_ctx_3->is_raft_leader()); EXPECT_TRUE(repl_ctx_3 && repl_ctx_3->raft_leader_id() == to_string(app_1_->id_)); - EXPECT_TRUE(repl_ctx && repl_ctx_3->get_raft_status().size() == 0); + EXPECT_TRUE(repl_ctx && repl_ctx_3->get_raft_status().size() == 1); std::list< nuraft_mesg::replica_config > cluster_config; repl_ctx->get_cluster_config(cluster_config);