Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(FQDN): Minor refactor on remove_node() #1994

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

refactor(FQDN): Minor refactor on remove_node() #1994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions src/common/replication_other_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 2 additions & 5 deletions src/meta/test/meta_partition_guardian_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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;

Expand Down
6 changes: 2 additions & 4 deletions src/meta/test/misc/misc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 2 additions & 6 deletions src/meta/test/update_configuration_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
* THE SOFTWARE.
*/

// IWYU pragma: no_include <ext/alloc_traits.h>
#include <algorithm>
#include <atomic>
#include <chrono>
#include <cstdint>
Expand Down Expand Up @@ -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:
Expand All @@ -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;

Expand Down
19 changes: 6 additions & 13 deletions src/replica/replica_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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(
Expand All @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/replica/replica_stub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
18 changes: 18 additions & 0 deletions src/rpc/rpc_host_port.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <netdb.h>
#include <netinet/in.h>
#include <sys/socket.h>
#include <algorithm>
#include <cstring>
#include <memory>
#include <unordered_set>
Expand Down Expand Up @@ -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
13 changes: 13 additions & 0 deletions src/rpc/rpc_host_port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
Loading