Skip to content

Commit a260510

Browse files
acelyc111acelyc111
committed
refactor(FQDN): Minor refactor on remove_node()
1 parent 2bd86c1 commit a260510

File tree

8 files changed

+44
-38
lines changed

8 files changed

+44
-38
lines changed

‎src/common/replication_other_types.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,17 +95,6 @@ inline bool is_partition_config_equal(const partition_configuration &pc1,
9595
class replica_helper
9696
{
9797
public:
98-
template <typename T>
99-
static bool remove_node(const T node,
100-
/*inout*/ std::vector<T> &nodes)
101-
{
102-
auto it = std::find(nodes.begin(), nodes.end(), node);
103-
if (it != nodes.end()) {
104-
nodes.erase(it);
105-
return true;
106-
}
107-
return false;
108-
}
10998
static bool get_replica_config(const partition_configuration &pc,
11099
const ::dsn::host_port &node,
111100
/*out*/ replica_configuration &rc);

‎src/meta/test/meta_partition_guardian_test.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,7 @@ static void apply_update_request(/*in-out*/ configuration_update_request &update
8282
case config_type::CT_ASSIGN_PRIMARY:
8383
case config_type::CT_UPGRADE_TO_PRIMARY:
8484
SET_OBJ_IP_AND_HOST_PORT(pc, primary, update_req, node);
85-
// TODO(yingchun): optimize the following code
86-
replica_helper::remove_node(update_req.node, pc.secondaries);
87-
replica_helper::remove_node(update_req.hp_node, pc.hp_secondaries);
85+
REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, update_req, node);
8886
break;
8987

9088
case config_type::CT_ADD_SECONDARY:
@@ -100,8 +98,7 @@ static void apply_update_request(/*in-out*/ configuration_update_request &update
10098
RESET_IP_AND_HOST_PORT(pc, primary);
10199
} else {
102100
CHECK_NE(update_req.node, pc.primary);
103-
replica_helper::remove_node(update_req.node, pc.secondaries);
104-
replica_helper::remove_node(update_req.hp_node, pc.hp_secondaries);
101+
REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, update_req, node);
105102
}
106103
break;
107104

‎src/meta/test/misc/misc.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,7 @@ void proposal_action_check_and_apply(const configuration_proposal_action &act,
360360
CHECK(node != nodes.end(), "");
361361
ns = &node->second;
362362
SET_IP_AND_HOST_PORT(pc, primary, act.node, hp_node);
363-
CHECK(replica_helper::remove_node(hp_node, pc.hp_secondaries), "");
364-
CHECK(replica_helper::remove_node(act.node, pc.secondaries), "");
363+
CHECK(REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, act, node), "");
365364
ns->put_partition(pc.pid, true);
366365
break;
367366
}
@@ -391,8 +390,7 @@ void proposal_action_check_and_apply(const configuration_proposal_action &act,
391390
CHECK_EQ(pc.primary, act.target);
392391
CHECK(is_secondary(pc, hp_node), "");
393392
CHECK(is_secondary(pc, act.node), "");
394-
CHECK(replica_helper::remove_node(hp_node, pc.hp_secondaries), "");
395-
CHECK(replica_helper::remove_node(act.node, pc.secondaries), "");
393+
CHECK(REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, act, node), "");
396394
const auto node = nodes.find(hp_node);
397395
CHECK(node != nodes.end(), "");
398396
ns = &node->second;

‎src/meta/test/update_configuration_test.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,7 @@ class fake_sender_meta_service : public dsn::replication::meta_service
107107
case config_type::CT_ASSIGN_PRIMARY:
108108
case config_type::CT_UPGRADE_TO_PRIMARY:
109109
SET_OBJ_IP_AND_HOST_PORT(pc, primary, *update_req, node);
110-
replica_helper::remove_node(update_req->node, pc.secondaries);
111-
replica_helper::remove_node(update_req->hp_node, pc.hp_secondaries);
110+
REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, *update_req, node);
112111
break;
113112

114113
case config_type::CT_ADD_SECONDARY:
@@ -124,8 +123,7 @@ class fake_sender_meta_service : public dsn::replication::meta_service
124123
RESET_IP_AND_HOST_PORT(pc, primary);
125124
} else {
126125
CHECK_NE(update_req->node, pc.primary);
127-
replica_helper::remove_node(update_req->node, pc.secondaries);
128-
replica_helper::remove_node(update_req->hp_node, pc.hp_secondaries);
126+
REMOVE_IP_AND_HOST_PORT_BY_OBJ(pc, secondaries, *update_req, node);
129127
}
130128
break;
131129

‎src/replica/replica_config.cpp

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ void replica::assign_primary(configuration_update_request &proposal)
174174

175175
SET_IP_AND_HOST_PORT(
176176
proposal.config, primary, _stub->primary_address(), _stub->primary_host_port());
177-
replica_helper::remove_node(_stub->primary_address(), proposal.config.secondaries);
178-
replica_helper::remove_node(_stub->primary_host_port(), proposal.config.hp_secondaries);
177+
REMOVE_IP_AND_HOST_PORT(
178+
proposal.config, secondaries, _stub->primary_address(), _stub->primary_host_port());
179179

180180
update_configuration_on_meta_server(proposal.type, node, proposal.config);
181181
}
@@ -298,12 +298,9 @@ void replica::downgrade_to_inactive_on_primary(configuration_update_request &pro
298298
RESET_IP_AND_HOST_PORT(proposal.config, primary);
299299
} else {
300300
CHECK_NE(proposal.node, proposal.config.primary);
301-
CHECK(replica_helper::remove_node(proposal.node, proposal.config.secondaries),
301+
CHECK(REMOVE_IP_AND_HOST_PORT_BY_OBJ(proposal.config, secondaries, proposal, node),
302302
"remove node failed, node = {}",
303-
proposal.node);
304-
CHECK(replica_helper::remove_node(node, proposal.config.hp_secondaries),
305-
"remove node failed, node = {}",
306-
node);
303+
FMT_HOST_PORT_AND_IP(proposal, node));
307304
}
308305

309306
update_configuration_on_meta_server(
@@ -330,12 +327,9 @@ void replica::remove(configuration_update_request &proposal)
330327
RESET_IP_AND_HOST_PORT(proposal.config, primary);
331328
break;
332329
case partition_status::PS_SECONDARY: {
333-
CHECK(replica_helper::remove_node(proposal.node, proposal.config.secondaries),
330+
CHECK(REMOVE_IP_AND_HOST_PORT_BY_OBJ(proposal.config, secondaries, proposal, node),
334331
"remove node failed, node = {}",
335-
proposal.node);
336-
CHECK(replica_helper::remove_node(node, proposal.config.hp_secondaries),
337-
"remove_node failed, node = {}",
338-
node);
332+
FMT_HOST_PORT_AND_IP(proposal, node));
339333
} break;
340334
case partition_status::PS_POTENTIAL_SECONDARY:
341335
break;

‎src/replica/replica_stub.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,8 +1471,8 @@ void replica_stub::remove_replica_on_meta_server(const app_info &info,
14711471

14721472
if (_primary_host_port == pc.hp_primary) {
14731473
RESET_IP_AND_HOST_PORT(request->config, primary);
1474-
} else if (replica_helper::remove_node(primary_address(), request->config.secondaries) &&
1475-
replica_helper::remove_node(_primary_host_port, request->config.hp_secondaries)) {
1474+
} else if (REMOVE_IP_AND_HOST_PORT(
1475+
request->config, secondaries, primary_address(), _primary_host_port)) {
14761476
} else {
14771477
return;
14781478
}

‎src/rpc/rpc_host_port.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,4 +249,21 @@ error_s host_port::lookup_hostname(uint32_t ip, std::string *hostname)
249249
return error_s::ok();
250250
}
251251

252+
bool remove_node(const rpc_address &addr,
253+
const host_port &hp,
254+
/*inout*/ std::vector<rpc_address> &addrs,
255+
/*inout*/ std::vector<host_port> &hps)
256+
{
257+
const auto it_addr = std::find(addrs.begin(), addrs.end(), addr);
258+
const auto it_hp = std::find(hps.begin(), hps.end(), hp);
259+
bool found_addr = (it_addr != addrs.end());
260+
bool found_hp = (it_hp != hps.end());
261+
DCHECK_EQ(found_addr, found_hp);
262+
if (found_addr) {
263+
addrs.erase(it_addr);
264+
hps.erase(it_hp);
265+
}
266+
return found_addr;
267+
}
268+
252269
} // namespace dsn

‎src/rpc/rpc_host_port.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,14 @@ class TProtocol;
232232
DCHECK_EQ(_obj.field.size(), _obj.hp_##field.size()); \
233233
} while (0)
234234

235+
#define REMOVE_IP_AND_HOST_PORT(dst_obj, dst_field, to_rm_addr, to_rm_hp) \
236+
remove_node(to_rm_addr, to_rm_hp, dst_obj.dst_field, dst_obj.hp_##dst_field)
237+
#define REMOVE_IP_AND_HOST_PORT_BY_OBJ(dst_obj, dst_field, to_rm_obj, to_rm_field) \
238+
remove_node((to_rm_obj).to_rm_field, \
239+
(to_rm_obj).hp_##to_rm_field, \
240+
dst_obj.dst_field, \
241+
dst_obj.hp_##dst_field)
242+
235243
// TODO(yingchun): the 'hp' can be reduced.
236244
// Set 'value' to the '<field>' map and optional 'hp_<field>' map of 'obj'. The key of the
237245
// 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)
368376
return true;
369377
}
370378
}
379+
380+
bool remove_node(const rpc_address &addr,
381+
const host_port &hp,
382+
/*inout*/ std::vector<rpc_address> &addrs,
383+
/*inout*/ std::vector<host_port> &hps);
371384
} // namespace dsn
372385

373386
USER_DEFINED_STRUCTURE_FORMATTER(::dsn::host_port);

0 commit comments

Comments
 (0)