acelyc111 commented on code in PR #1658:
URL:
https://github.com/apache/incubator-pegasus/pull/1658#discussion_r1445719634
##########
src/runtime/test/rpc.cpp:
##########
@@ -238,7 +239,7 @@ TEST(core, send_to_invalid_address)
{
::dsn::rpc_address group = build_group();
/* here we assume 10.255.254.253:32766 is not assigned */
Review Comment:
The comment is incorrect now, remove it.
##########
src/meta/cluster_balance_policy.cpp:
##########
@@ -544,10 +545,17 @@ bool cluster_balance_policy::apply_move(const move_info
&move,
// add into migration list and selected_pid
partition_configuration pc;
pc.pid = move.pid;
- pc.primary = primary_addr;
- list[move.pid] = generate_balancer_request(*_global_view->apps, pc,
move.type, source, target);
+ pc.hp_primary = primary_hp;
+ std::shared_ptr<dsn::dns_resolver> resolver;
+ resolver = _svc->get_dns_resolver();
+ auto source_addr = resolver->resolve_address(source);
+ auto target_addr = resolver->resolve_address(target);
Review Comment:
```suggestion
auto source_addr = _svc->get_dns_resolver()->resolve_address(source);
auto target_addr = _svc->get_dns_resolver()->resolve_address(target);
```
##########
src/nfs/test/main.cpp:
##########
@@ -68,7 +69,8 @@ INSTANTIATE_TEST_CASE_P(, nfs_test, ::testing::Values(false,
true));
TEST_P(nfs_test, basic)
{
- auto nfs = dsn::nfs_node::create();
+ std::shared_ptr<dns_resolver> resolver = std::make_shared<dns_resolver>();
Review Comment:
```suggestion
auto resolver = std::make_shared<dns_resolver>();
```
##########
src/replica/replica_2pc.cpp:
##########
@@ -306,9 +308,10 @@ void replica::init_prepare(mutation_ptr &mu, bool
reconciliation, bool pop_all_c
// remote prepare
mu->set_prepare_ts();
- mu->set_left_secondary_ack_count((unsigned
int)_primary_states.membership.secondaries.size());
- for (auto it = _primary_states.membership.secondaries.begin();
- it != _primary_states.membership.secondaries.end();
+ mu->set_left_secondary_ack_count(
+ (unsigned int)_primary_states.membership.hp_secondaries.size());
+ for (auto it = _primary_states.membership.hp_secondaries.begin();
+ it != _primary_states.membership.hp_secondaries.end();
++it) {
send_prepare_message(*it,
Review Comment:
```suggestion
for (const auto &secondary : hp_secondaries) {
send_prepare_message(secondary,
```
##########
src/replica/duplication/test/replica_follower_test.cpp:
##########
@@ -104,7 +106,8 @@ class replica_follower_test : public duplication_test_base
void init_nfs()
{
- stub->_nfs = nfs_node::create();
+ std::shared_ptr<dns_resolver> resolver =
std::make_shared<dns_resolver>();
Review Comment:
Because we can infer the type from std::make_shared<xxx>, so we can use auto
simply.
```suggestion
auto resolver = std::make_shared<dns_resolver>();
```
##########
src/redis_protocol/proxy_lib/proxy_layer.h:
##########
@@ -80,7 +80,8 @@ class proxy_session : public
std::enable_shared_from_this<proxy_session>
// we need to backup one request to create a response struct.
dsn::message_ex *_backup_one_request;
// the client address for which this session served
- dsn::rpc_address _remote_address;
+ dsn::host_port _remote_host_port;
+ std::string _host_port;
Review Comment:
The two member names are confused, how about improving it by renaming
_host_port to _log_prefix ?
##########
src/meta/test/copy_replica_operation_test.cpp:
##########
@@ -166,9 +168,11 @@ TEST(copy_primary_operation, can_select)
{
app_mapper apps;
node_mapper nodes;
- std::vector<dsn::rpc_address> address_vec;
- std::unordered_map<dsn::rpc_address, int> address_id;
- copy_primary_operation op(nullptr, apps, nodes, address_vec, address_id,
false, false);
+ std::vector<dsn::host_port> host_port_vec;
+ std::unordered_map<dsn::host_port, int> host_port_id;
+ std::shared_ptr<dns_resolver> resolver = std::make_shared<dns_resolver>();
Review Comment:
```suggestion
auto resolver = std::make_shared<dns_resolver>();
```
##########
src/nfs/test/main.cpp:
##########
@@ -68,7 +69,8 @@ INSTANTIATE_TEST_CASE_P(, nfs_test, ::testing::Values(false,
true));
TEST_P(nfs_test, basic)
{
- auto nfs = dsn::nfs_node::create();
+ std::shared_ptr<dns_resolver> resolver = std::make_shared<dns_resolver>();
+ std::unique_ptr<dsn::nfs_node> nfs(dsn::nfs_node::create(resolver));
Review Comment:
```suggestion
auto nfs = dsn::nfs_node::create(resolver);
```
##########
src/meta/test/copy_replica_operation_test.cpp:
##########
@@ -183,9 +187,11 @@ TEST(copy_primary_operation, only_copy_primary)
{
app_mapper apps;
node_mapper nodes;
- std::vector<dsn::rpc_address> address_vec;
- std::unordered_map<dsn::rpc_address, int> address_id;
- copy_primary_operation op(nullptr, apps, nodes, address_vec, address_id,
false, false);
+ std::vector<dsn::host_port> host_port_vec;
+ std::unordered_map<dsn::host_port, int> host_port_id;
+ std::shared_ptr<dns_resolver> resolver = std::make_shared<dns_resolver>();
Review Comment:
```suggestion
auto resolver = std::make_shared<dns_resolver>();
```
##########
src/meta/test/meta_duplication_service_test.cpp:
##########
@@ -127,11 +129,13 @@ class meta_duplication_service_test : public
meta_test_base
}
duplication_sync_response
- duplication_sync(const rpc_address &node,
+ duplication_sync(const rpc_address &addr,
Review Comment:
This is only used in tests, is it necessary to keep the rpc_address
parameter?
##########
src/meta/test/copy_replica_operation_test.cpp:
##########
@@ -216,13 +222,14 @@ TEST(copy_secondary_operation, misc)
node_state ns3;
nodes[addr3] = ns3;
- std::vector<dsn::rpc_address> address_vec{addr1, addr2, addr3};
- std::unordered_map<dsn::rpc_address, int> address_id;
- address_id[addr1] = 0;
- address_id[addr2] = 1;
- address_id[addr3] = 2;
- copy_secondary_operation op(app, apps, nodes, address_vec, address_id, 0);
- op.init_ordered_address_ids();
+ std::vector<dsn::host_port> host_port_vec{addr1, addr2, addr3};
+ std::unordered_map<dsn::host_port, int> host_port_id;
+ host_port_id[addr1] = 0;
+ host_port_id[addr2] = 1;
+ host_port_id[addr3] = 2;
+ std::shared_ptr<dns_resolver> resolver = std::make_shared<dns_resolver>();
Review Comment:
```suggestion
auto resolver = std::make_shared<dns_resolver>();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
