refactor(FQDN): Minor refactor on remove_node()

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 <typename T>
-    static bool remove_node(const T node,
-                            /*inout*/ std::vector<T> &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);

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;
 

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;

src/meta/test/update_configuration_test.cpp

@@ -24,8 +24,6 @@
  * THE SOFTWARE.
  */
 
-// IWYU pragma: no_include <ext/alloc_traits.h>
-#include <algorithm>
 #include <atomic>
 #include <chrono>
 #include <cstdint>
@@ -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;
 

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;
     }

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;
     }

src/rpc/rpc_host_port.cpp

@@ -22,6 +22,7 @@
 #include <netdb.h>
 #include <netinet/in.h>
 #include <sys/socket.h>
+#include <algorithm>
 #include <cstring>
 #include <memory>
 #include <unordered_set>
@@ -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<rpc_address> &addrs,
+                 /*inout*/ std::vector<host_port> &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

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 '<field>' map and optional 'hp_<field>' 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<rpc_address> &addrs,
+                 /*inout*/ std::vector<host_port> &hps);
 } // namespace dsn
 
 USER_DEFINED_STRUCTURE_FORMATTER(::dsn::host_port);