temp cr

run.sh

@@ -486,15 +486,15 @@ function run_test()
             run_start_zk
         fi
         pushd ${BUILD_LATEST_DIR}/bin/${module}
-	local function_tests=(
+        local function_tests=(
           backup_restore_test
-	  recovery_test
-	  restore_test
-	  base_api_test
-	  throttle_test
-	  bulk_load_test
-	  detect_hotspot_test
-	  partition_split_test
+	      recovery_test
+	      restore_test
+	      base_api_test
+	      throttle_test
+	      bulk_load_test
+	      detect_hotspot_test
+	      partition_split_test
         )
         if [[ "${function_tests[@]}"  =~ "${test_modules}" ]]; then
           sed -i "s/@LOCAL_HOSTNAME@/${LOCAL_HOSTNAME}/g"  ./config.ini

src/client/partition_resolver_simple.cpp

@@ -416,7 +416,7 @@ void partition_resolver_simple::handle_pending_requests(std::deque<request_conte
 /*search in cache*/
 host_port partition_resolver_simple::get_host_port(const partition_configuration &config) const
 {
-    auto pc = config;
+    const auto &pc = config;
     if (_app_is_stateful) {
         return pc.hp_primary;
     } else {

src/client/replication_ddl_client.cpp

@@ -527,13 +527,7 @@ dsn::error_code replication_ddl_client::list_nodes(
 
     for (dsn::replication::node_info &n : resp.infos) {
         host_port hp;
-        if (n.__isset.hp_address) {
-            hp = n.hp_address;
-            LOG_ERROR("lgh: replica_server hp {}", hp);
-        } else {
-            hp = host_port(n.address);
-            LOG_ERROR("lgh: replica_server addr {} to hp {}", n.address, hp);
-        }
+	HOST_PORT(n, address, hp);
         nodes[hp] = n.status;
     }
 

src/common/replication_common.cpp

@@ -168,23 +168,23 @@ int32_t replication_options::app_mutation_2pc_min_replica_count(int32_t app_max_
 }
 
 /*static*/ bool replica_helper::remove_node(::dsn::host_port node,
-                                            /*inout*/ std::vector<::dsn::host_port> &nodeList)
+                                            /*inout*/ std::vector<::dsn::host_port> &nodes)
 {
-    auto it = std::find(nodeList.begin(), nodeList.end(), node);
-    if (it != nodeList.end()) {
-        nodeList.erase(it);
+    auto it = std::find(nodes.begin(), nodes.end(), node);
+    if (it != nodes.end()) {
+        nodes.erase(it);
         return true;
     } else {
         return false;
     }
 }
 
 /*static*/ bool replica_helper::remove_node(::dsn::rpc_address node,
-                                            /*inout*/ std::vector<::dsn::rpc_address> &nodeList)
+                                            /*inout*/ std::vector<::dsn::rpc_address> &nodes)
 {
-    auto it = std::find(nodeList.begin(), nodeList.end(), node);
-    if (it != nodeList.end()) {
-        nodeList.erase(it);
+    auto it = std::find(nodes.begin(), nodes.end(), node);
+    if (it != nodes.end()) {
+        nodes.erase(it);
         return true;
     } else {
         return false;

src/failure_detector/failure_detector.cpp

@@ -356,16 +356,8 @@ std::string failure_detector::get_allow_list(const std::vector<std::string> &arg
 void failure_detector::on_ping_internal(const beacon_msg &beacon, /*out*/ beacon_ack &ack)
 {
     host_port hp_from_addr, hp_to_addr;
-    if (beacon.__isset.hp_from_addr) {
-        hp_from_addr = beacon.hp_from_addr;
-    } else {
-        hp_from_addr = host_port(beacon.from_addr);
-    }
-    if (beacon.__isset.hp_to_addr) {
-        hp_to_addr = beacon.hp_to_addr;
-    } else {
-        hp_to_addr = host_port(beacon.to_addr);
-    }
+    HOST_PORT(beacon, from_addr, hp_from_addr);
+    HOST_PORT(beacon, to_addr, hp_from_addr);
 
     ack.time = beacon.time;
     ack.this_node = beacon.to_addr;
@@ -432,16 +424,8 @@ bool failure_detector::end_ping_internal(::dsn::error_code err, const beacon_ack
      * the caller of the end_ping_internal should lock necessarily!!!
      */
     host_port hp_this_node, hp_primary_node;
-    if (ack.__isset.hp_this_node) {
-        hp_this_node = ack.hp_this_node;
-    } else {
-        hp_this_node = host_port(ack.this_node);
-    }
-    if (ack.__isset.hp_primary_node) {
-        hp_primary_node = ack.hp_primary_node;
-    } else {
-        hp_primary_node = host_port(ack.primary_node);
-    }
+    HOST_PORT(ack, this_node, hp_this_node);
+    HOST_PORT(ack, primary_node, hp_primary_node);
 
     uint64_t beacon_send_time = ack.time;
 
@@ -457,9 +441,10 @@ bool failure_detector::end_ping_internal(::dsn::error_code err, const beacon_ack
 
     if (itr == _masters.end()) {
         LOG_WARNING("received beacon ack without corresponding master, ignore it, "
-                    "remote_master[{}({})], local_worker[{}]",
+                    "remote_master[{}({})], local_worker[{}({})]",
                     hp_this_node,
                     ack.this_node,
+                    dsn_primary_host_port(),
                     dsn_primary_address());
         return false;
     }

src/failure_detector/failure_detector_multimaster.cpp

@@ -82,16 +82,8 @@ void slave_failure_detector_with_multimaster::end_ping(::dsn::error_code err,
                                                        void *)
 {
     host_port hp_this_node, hp_primary_node;
-    if (ack.__isset.hp_this_node) {
-        hp_this_node = ack.hp_this_node;
-    } else {
-        hp_this_node = host_port(ack.this_node);
-    }
-    if (ack.__isset.hp_primary_node) {
-        hp_primary_node = ack.hp_primary_node;
-    } else {
-        hp_primary_node = host_port(ack.primary_node);
-    }
+    HOST_PORT(ack, this_node, hp_this_node);
+    HOST_PORT(ack, primary_node, hp_primary_node);
 
     LOG_INFO(
         "end ping result, error[{}], time[{}], ack.this_node[{}({})], ack.primary_node[{}({})], "

src/failure_detector/test/failure_detector.cpp

@@ -215,11 +215,7 @@ class test_worker : public service_app, public serverlet<test_worker>
                   request.is_register ? "reg" : "unreg");
 
         host_port master;
-        if (request.__isset.hp_master) {
-            master = request.hp_master;
-        } else {
-            master = host_port(request.master);
-        }
+	HOST_PORT(request, master, master);
 
         if (request.is_register)
             _worker_fd->register_master(master);

src/meta/greedy_load_balancer.cpp

@@ -163,7 +163,7 @@ void greedy_load_balancer::score(meta_view view, double &primary_stddev, double
 
 bool greedy_load_balancer::all_replica_infos_collected(const node_state &ns)
 {
-    auto n = ns.host_port();
+    const auto &n = ns.host_port();
     return ns.for_each_partition([this, n](const dsn::gpid &pid) {
         config_context &cc = *get_config_context(*(t_global_view->apps), pid);
         if (cc.find_from_serving(n) == cc.serving.end()) {

src/meta/meta_server_failure_detector.cpp

@@ -228,11 +228,7 @@ bool meta_server_failure_detector::update_stability_stat(const fd::beacon_msg &b
     zauto_lock l(_map_lock);
 
     host_port hp_from_addr;
-    if (beacon.__isset.hp_from_addr) {
-        hp_from_addr = beacon.hp_from_addr;
-    } else {
-        hp_from_addr = host_port(beacon.from_addr);
-    }
+    HOST_PORT(beacon, from_addr, hp_from_addr);
 
     auto iter = _stablity.find(hp_from_addr);
     if (iter == _stablity.end()) {
@@ -290,16 +286,8 @@ void meta_server_failure_detector::on_ping(const fd::beacon_msg &beacon,
                                            rpc_replier<fd::beacon_ack> &reply)
 {
     host_port hp_from_addr, hp_to_addr;
-    if (beacon.__isset.hp_from_addr) {
-        hp_from_addr = beacon.hp_from_addr;
-    } else {
-        hp_from_addr = host_port(beacon.from_addr);
-    }
-    if (beacon.__isset.hp_to_addr) {
-        hp_to_addr = beacon.hp_to_addr;
-    } else {
-        hp_to_addr = host_port(beacon.to_addr);
-    }
+    HOST_PORT(beacon, from_addr, hp_from_addr);
+    HOST_PORT(beacon, to_addr, hp_to_addr);
 
     if (beacon.__isset.start_time && !update_stability_stat(beacon)) {
         LOG_WARNING(

src/meta/server_state.cpp

@@ -850,7 +850,8 @@ void server_state::on_config_sync(configuration_query_by_node_rpc rpc)
     bool reject_this_request = false;
     response.__isset.gc_replicas = false;
 
-    host_port hp_node = request.__isset.hp_node ? request.hp_node : host_port(request.node);
+    host_port hp_node;
+    HOST_PORT(request, node, hp_node);
 
     LOG_INFO("got config sync request from {}({}), stored_replicas_count({})",
              hp_node,
@@ -1486,11 +1487,7 @@ void server_state::list_apps(const configuration_list_apps_request &request,
 void server_state::send_proposal(host_port target, const configuration_update_request &proposal)
 {
     host_port hp;
-    if (proposal.__isset.hp_node) {
-        hp = proposal.hp_node;
-    } else {
-        hp = host_port(proposal.node);
-    }
+    HOST_PORT(proposal, node, hp);
     LOG_INFO("send proposal {} for gpid({}), ballot = {}, target = {}, node = {}({})",
              ::dsn::enum_to_string(proposal.type),
              proposal.config.pid,

src/meta/test/misc/misc.cpp

@@ -243,12 +243,8 @@ void track_disk_info_check_and_apply(const dsn::replication::configuration_propo
     CHECK_NOTNULL(cc, "");
 
     dsn::host_port hp_target, hp_node;
-    if (act.__isset.hp_target) {
-        hp_target = dsn::host_port(act.target);
-    }
-    if (act.__isset.hp_node) {
-        hp_node = dsn::host_port(act.node);
-    }
+    HOST_PORT(act, target, hp_target);
+    HOST_PORT(act, node, hp_node);
 
     fs_manager *target_manager = get_fs_manager(manager, hp_target);
     CHECK_NOTNULL(target_manager, "");
@@ -308,12 +304,8 @@ void proposal_action_check_and_apply(const configuration_proposal_action &act,
     }
 
     dsn::host_port hp_target, hp_node;
-    if (act.__isset.hp_target) {
-        hp_target = dsn::host_port(act.target);
-    }
-    if (act.__isset.hp_node) {
-        hp_node = dsn::host_port(act.node);
-    }
+    HOST_PORT(act, target, hp_target);
+    HOST_PORT(act, node, hp_node);
 
     switch (act.type) {
     case config_type::CT_ASSIGN_PRIMARY:

src/replica/duplication/replica_follower.cpp

@@ -225,11 +225,7 @@ error_code replica_follower::nfs_copy_checkpoint(error_code err, learn_response
     }
 
     host_port hp;
-    if (resp.__isset.hp_address) {
-        hp = resp.hp_address;
-    } else {
-        hp = host_port(resp.address);
-    }
+    HOST_PORT(resp, address, hp);
 
     nfs_copy_remote_files(hp, resp.replica_disk_tag, resp.base_local_dir, resp.state.files, dest);
     return ERR_OK;

src/replica/replica_learn.cpp

@@ -384,11 +384,7 @@ void replica::on_learn(dsn::message_ex *msg, const learn_request &request)
     _primary_states.get_replica_config(partition_status::PS_POTENTIAL_SECONDARY, response.config);
 
     host_port hp_learner;
-    if (request.__isset.hp_learner) {
-        hp_learner = request.hp_learner;
-    } else {
-        hp_learner = host_port(request.learner);
-    }
+    HOST_PORT(request, learner, hp_learner);
 
     auto it = _primary_states.learners.find(hp_learner);
     if (it == _primary_states.learners.end()) {
@@ -1386,11 +1382,7 @@ void replica::on_learn_completion_notification(const group_check_response &repor
     _checker.only_one_thread_access();
 
     host_port hp;
-    if (report.__isset.hp_node) {
-        hp = report.hp_node;
-    } else {
-        hp = host_port(report.node);
-    }
+    HOST_PORT(report, node, hp);
 
     LOG_INFO_PREFIX(
         "on_learn_completion_notification[{:#018x}]: learner = {}({}), learning_status = {}",

src/replica/split/replica_split_manager.cpp

@@ -652,11 +652,7 @@ void replica_split_manager::parent_handle_child_catch_up(
     }
 
     host_port hp;
-    if (request.__isset.hp_child_address) {
-        hp = request.hp_child_address;
-    } else {
-        hp = host_port(request.child_address);
-    }
+    HOST_PORT(request, child_address, hp);
 
     response.err = ERR_OK;
     LOG_INFO_PREFIX("receive catch_up request from {}@{}({}), current ballot={}",

src/runtime/rpc/rpc_host_port.h

@@ -41,6 +41,15 @@ class TProtocol;
 } // namespace thrift
 } // namespace apache
 
+#define HOST_PORT(obj, field, target)                 \
+    do {                                              \
+        if (obj.__isset.hp_##field) {                 \
+            target = obj.hp_##field;                  \
+        } else {                                      \
+            target = std::move(dsn::host_port(obj.field)); \
+        }                                             \
+    } while (0)
+
 namespace dsn {
 
 class rpc_group_host_port;

src/shell/commands/recovery.cpp

@@ -296,11 +296,7 @@ bool ddd_diagnose(command_executor *e, shell_context *sc, arguments args)
         int j = 0;
         for (const ddd_node_info &n : pinfo.dropped) {
             dsn::host_port hp;
-            if (n.__isset.hp_node) {
-                hp = n.hp_node;
-            } else {
-                hp = dsn::host_port(n.node);
-            }
+	    HOST_PORT(n, node, hp);
             char time_buf[30] = {0};
             ::dsn::utils::time_ms_to_string(n.drop_time_ms, time_buf);
             out << "    dropped[" << j++ << "]: "