From 07e55a9a21b34316bad877c607efbfd9570e9734 Mon Sep 17 00:00:00 2001 From: Yingchun Lai Date: Mon, 29 Apr 2024 16:10:43 +0800 Subject: [PATCH] refactor(FQDN): Minor refactor on remove_node() --- src/common/replication_other_types.h | 11 ----------- .../test/meta_partition_guardian_test.cpp | 7 ++----- src/meta/test/misc/misc.cpp | 6 ++---- src/meta/test/update_configuration_test.cpp | 8 ++------ src/replica/replica_config.cpp | 19 ++++++------------- src/replica/replica_stub.cpp | 4 ++-- src/rpc/rpc_host_port.cpp | 18 ++++++++++++++++++ src/rpc/rpc_host_port.h | 13 +++++++++++++ 8 files changed, 45 insertions(+), 41 deletions(-) diff --git a/src/common/replication_other_types.h b/src/common/replication_other_types.h index e23164a187..288a6615af 100644 --- a/src/common/replication_other_types.h +++ b/src/common/replication_other_types.h @@ -95,17 +95,6 @@ inline bool is_partition_config_equal(const partition_configuration &pc1, class replica_helper { public: - template - static bool remove_node(const T node, - /*inout*/ std::vector &nodes) - { - auto it = std::find(nodes.begin(), nodes.end(), node); - if (it != nodes.end()) { - nodes.erase(it); - return true; - } - return false; - } static bool get_replica_config(const partition_configuration &pc, const ::dsn::host_port &node, /*out*/ replica_configuration &rc); diff --git a/src/meta/test/meta_partition_guardian_test.cpp b/src/meta/test/meta_partition_guardian_test.cpp index 931433a754..1ca4a72089 100644 --- a/src/meta/test/meta_partition_guardian_test.cpp +++ b/src/meta/test/meta_partition_guardian_test.cpp @@ -82,9 +82,7 @@ static void apply_update_request(/*in-out*/ configuration_update_request &update case config_type::CT_ASSIGN_PRIMARY: case config_type::CT_UPGRADE_TO_PRIMARY: SET_OBJ_IP_AND_HOST_PORT(pc, primary, update_req, node); - // TODO(yingchun): optimize the following code - replica_helper::remove_node(update_req.node, pc.secondaries); - replica_helper::remove_node(update_req.hp_node, pc.hp_secondaries); + REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, update_req, node); break; case config_type::CT_ADD_SECONDARY: @@ -100,8 +98,7 @@ static void apply_update_request(/*in-out*/ configuration_update_request &update RESET_IP_AND_HOST_PORT(pc, primary); } else { CHECK_NE(update_req.node, pc.primary); - replica_helper::remove_node(update_req.node, pc.secondaries); - replica_helper::remove_node(update_req.hp_node, pc.hp_secondaries); + REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, update_req, node); } break; diff --git a/src/meta/test/misc/misc.cpp b/src/meta/test/misc/misc.cpp index a3bd371445..ca52a33847 100644 --- a/src/meta/test/misc/misc.cpp +++ b/src/meta/test/misc/misc.cpp @@ -360,8 +360,7 @@ void proposal_action_check_and_apply(const configuration_proposal_action &act, CHECK(node != nodes.end(), ""); ns = &node->second; SET_IP_AND_HOST_PORT(pc, primary, act.node, hp_node); - CHECK(replica_helper::remove_node(hp_node, pc.hp_secondaries), ""); - CHECK(replica_helper::remove_node(act.node, pc.secondaries), ""); + CHECK(REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, act, node), ""); ns->put_partition(pc.pid, true); break; } @@ -391,8 +390,7 @@ void proposal_action_check_and_apply(const configuration_proposal_action &act, CHECK_EQ(pc.primary, act.target); CHECK(is_secondary(pc, hp_node), ""); CHECK(is_secondary(pc, act.node), ""); - CHECK(replica_helper::remove_node(hp_node, pc.hp_secondaries), ""); - CHECK(replica_helper::remove_node(act.node, pc.secondaries), ""); + CHECK(REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, act, node), ""); const auto node = nodes.find(hp_node); CHECK(node != nodes.end(), ""); ns = &node->second; diff --git a/src/meta/test/update_configuration_test.cpp b/src/meta/test/update_configuration_test.cpp index c0ef9b0d63..195459febf 100644 --- a/src/meta/test/update_configuration_test.cpp +++ b/src/meta/test/update_configuration_test.cpp @@ -24,8 +24,6 @@ * THE SOFTWARE. */ -// IWYU pragma: no_include -#include #include #include #include @@ -107,8 +105,7 @@ class fake_sender_meta_service : public dsn::replication::meta_service case config_type::CT_ASSIGN_PRIMARY: case config_type::CT_UPGRADE_TO_PRIMARY: SET_OBJ_IP_AND_HOST_PORT(pc, primary, *update_req, node); - replica_helper::remove_node(update_req->node, pc.secondaries); - replica_helper::remove_node(update_req->hp_node, pc.hp_secondaries); + REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, *update_req, node); break; case config_type::CT_ADD_SECONDARY: @@ -124,8 +121,7 @@ class fake_sender_meta_service : public dsn::replication::meta_service RESET_IP_AND_HOST_PORT(pc, primary); } else { CHECK_NE(update_req->node, pc.primary); - replica_helper::remove_node(update_req->node, pc.secondaries); - replica_helper::remove_node(update_req->hp_node, pc.hp_secondaries); + REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, *update_req, node); } break; diff --git a/src/replica/replica_config.cpp b/src/replica/replica_config.cpp index 8fc73eec71..21d9bf2d0e 100644 --- a/src/replica/replica_config.cpp +++ b/src/replica/replica_config.cpp @@ -174,8 +174,8 @@ void replica::assign_primary(configuration_update_request &proposal) SET_IP_AND_HOST_PORT( proposal.config, primary, _stub->primary_address(), _stub->primary_host_port()); - replica_helper::remove_node(_stub->primary_address(), proposal.config.secondaries); - replica_helper::remove_node(_stub->primary_host_port(), proposal.config.hp_secondaries); + REMOVE_IP_AND_HOST_PORT( + proposal.config, secondaries, _stub->primary_address(), _stub->primary_host_port()); update_configuration_on_meta_server(proposal.type, node, proposal.config); } @@ -298,12 +298,9 @@ void replica::downgrade_to_inactive_on_primary(configuration_update_request &pro RESET_IP_AND_HOST_PORT(proposal.config, primary); } else { CHECK_NE(proposal.node, proposal.config.primary); - CHECK(replica_helper::remove_node(proposal.node, proposal.config.secondaries), + CHECK(REMOVE_IP_AND_HOST_PORT_BY_OBJ(proposal.config, secondaries, proposal, node), "remove node failed, node = {}", - proposal.node); - CHECK(replica_helper::remove_node(node, proposal.config.hp_secondaries), - "remove node failed, node = {}", - node); + FMT_HOST_PORT_AND_IP(proposal, node)); } update_configuration_on_meta_server( @@ -330,15 +327,11 @@ void replica::remove(configuration_update_request &proposal) RESET_IP_AND_HOST_PORT(proposal.config, primary); break; case partition_status::PS_SECONDARY: { - CHECK(replica_helper::remove_node(proposal.node, proposal.config.secondaries), + CHECK(REMOVE_IP_AND_HOST_PORT_BY_OBJ(proposal.config, secondaries, proposal, node), "remove node failed, node = {}", - proposal.node); - CHECK(replica_helper::remove_node(node, proposal.config.hp_secondaries), - "remove_node failed, node = {}", - node); + FMT_HOST_PORT_AND_IP(proposal, node)); } break; case partition_status::PS_POTENTIAL_SECONDARY: - break; default: break; } diff --git a/src/replica/replica_stub.cpp b/src/replica/replica_stub.cpp index de2b789730..b179e70f33 100644 --- a/src/replica/replica_stub.cpp +++ b/src/replica/replica_stub.cpp @@ -1471,8 +1471,8 @@ void replica_stub::remove_replica_on_meta_server(const app_info &info, if (_primary_host_port == pc.hp_primary) { RESET_IP_AND_HOST_PORT(request->config, primary); - } else if (replica_helper::remove_node(primary_address(), request->config.secondaries) && - replica_helper::remove_node(_primary_host_port, request->config.hp_secondaries)) { + } else if (REMOVE_IP_AND_HOST_PORT( + request->config, secondaries, primary_address(), _primary_host_port)) { } else { return; } diff --git a/src/rpc/rpc_host_port.cpp b/src/rpc/rpc_host_port.cpp index d7e4bd2b60..3293fff473 100644 --- a/src/rpc/rpc_host_port.cpp +++ b/src/rpc/rpc_host_port.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -249,4 +250,21 @@ error_s host_port::lookup_hostname(uint32_t ip, std::string *hostname) return error_s::ok(); } +bool remove_node(const rpc_address &addr, + const host_port &hp, + /*inout*/ std::vector &addrs, + /*inout*/ std::vector &hps) +{ + const auto it_addr = std::find(addrs.begin(), addrs.end(), addr); + const auto it_hp = std::find(hps.begin(), hps.end(), hp); + bool found_addr = (it_addr != addrs.end()); + bool found_hp = (it_hp != hps.end()); + DCHECK_EQ(found_addr, found_hp); + if (found_addr) { + addrs.erase(it_addr); + hps.erase(it_hp); + } + return found_addr; +} + } // namespace dsn diff --git a/src/rpc/rpc_host_port.h b/src/rpc/rpc_host_port.h index 42685efe86..bd6ce13494 100644 --- a/src/rpc/rpc_host_port.h +++ b/src/rpc/rpc_host_port.h @@ -232,6 +232,14 @@ class TProtocol; DCHECK_EQ(_obj.field.size(), _obj.hp_##field.size()); \ } while (0) +#define REMOVE_IP_AND_HOST_PORT(dst_obj, dst_field, to_rm_addr, to_rm_hp) \ + remove_node(to_rm_addr, to_rm_hp, dst_obj.dst_field, dst_obj.hp_##dst_field) +#define REMOVE_IP_AND_HOST_PORT_BY_OBJ(dst_obj, dst_field, to_rm_obj, to_rm_field) \ + remove_node((to_rm_obj).to_rm_field, \ + (to_rm_obj).hp_##to_rm_field, \ + dst_obj.dst_field, \ + dst_obj.hp_##dst_field) + // TODO(yingchun): the 'hp' can be reduced. // Set 'value' to the '' map and optional 'hp_' map of 'obj'. The key of the // maps are rpc_address and host_port type and indexed by 'addr' and 'hp', respectively. @@ -368,6 +376,11 @@ inline bool operator<(const host_port &hp1, const host_port &hp2) return true; } } + +bool remove_node(const rpc_address &addr, + const host_port &hp, + /*inout*/ std::vector &addrs, + /*inout*/ std::vector &hps); } // namespace dsn USER_DEFINED_STRUCTURE_FORMATTER(::dsn::host_port);