acelyc111 commented on code in PR #1658:
URL:
https://github.com/apache/incubator-pegasus/pull/1658#discussion_r1439223636
##########
src/test/function_test/recovery/test_recovery.cpp:
##########
@@ -211,7 +216,7 @@ TEST_F(recovery_test, recovery)
std::this_thread::sleep_for(std::chrono::seconds(10));
// recovery only from 1 & 2
- std::vector<dsn::rpc_address> nodes = get_rpc_address_list({34801,
34802});
+ std::vector<dsn::host_port> nodes = get_rpc_host_port_list({34801,
34802});
Review Comment:
```suggestion
std::vector<dsn::host_port> nodes = get_rpc_host_port_list({34801,
34802});
```
```suggestion
auto nodes = get_rpc_host_port_list({34801, 34802});
```
##########
src/test/function_test/base_api/test_batch_get.cpp:
##########
@@ -48,8 +49,9 @@ class batch_get : public test_util
TEST_F(batch_get, set_and_then_batch_get)
{
- auto rrdb_client =
- new ::dsn::apps::rrdb_client(kClusterName.c_str(), meta_list_,
table_name_.c_str());
+ std::shared_ptr<dsn::dns_resolver> dns_resolver =
std::make_shared<dsn::dns_resolver>();
Review Comment:
```suggestion
auto dns_resolver = std::make_shared<dsn::dns_resolver>();
```
##########
src/client/replication_ddl_client.cpp:
##########
@@ -776,17 +813,17 @@ dsn::error_code replication_ddl_client::list_app(const
std::string &app_name,
std::stringstream oss;
oss << replica_count << "/" << p.max_replica_count;
tp_details.append_data(oss.str());
- tp_details.append_data(
- (p.primary.is_invalid() ? "-" : host_name_resolve(resolve_ip,
-
p.primary.to_std_string())));
+ tp_details.append_data((p.hp_primary.is_invalid()
+ ? "-"
+ : host_name_resolve(resolve_ip,
p.hp_primary.to_string())));
Review Comment:
Because p.hp_primary is valid, we can use it directly, right?
##########
src/test/function_test/recovery/test_recovery.cpp:
##########
@@ -65,13 +66,17 @@ class recovery_test : public test_util
}
public:
- std::vector<dsn::rpc_address> get_rpc_address_list(const std::vector<int>
ports)
+ recovery_test() : test_util(std::map<std::string, std::string>(),
"single_master_cluster") {}
Review Comment:
It would be better to submit this change in a separated patch.
##########
src/shell/commands/detect_hotkey.cpp:
##########
@@ -100,10 +100,10 @@ bool detect_hotkey(command_executor *e, shell_context
*sc, arguments args)
return false;
}
- dsn::rpc_address target_address;
+ dsn::host_port target_node;
Review Comment:
Unify to naming `target_hp`?
##########
src/client/replication_ddl_client.cpp:
##########
@@ -490,8 +525,10 @@ dsn::error_code replication_ddl_client::list_nodes(
return resp.err;
}
- for (const dsn::replication::node_info &n : resp.infos) {
- nodes[n.address] = n.status;
+ for (dsn::replication::node_info &n : resp.infos) {
Review Comment:
```suggestion
for (const auto &n : resp.infos) {
```
##########
src/meta/app_balance_policy.cpp:
##########
@@ -182,8 +187,8 @@ bool copy_secondary_operation::can_select(gpid pid,
migration_list *result)
return false;
}
- int id_min = *_ordered_address_ids.begin();
- const node_state &min_ns = _nodes.at(_address_vec[id_min]);
+ int id_min = *_ordered_host_port_ids.begin();
+ const node_state &min_ns = _nodes.at(host_port(_host_port_vec[id_min]));
Review Comment:
```suggestion
const node_state &min_ns = _nodes.at(_host_port_vec[id_min]);
```
##########
src/meta/app_balance_policy.cpp:
##########
@@ -163,8 +168,8 @@ int copy_secondary_operation::get_partition_count(const
node_state &ns) const
bool copy_secondary_operation::can_select(gpid pid, migration_list *result)
{
- int id_max = *_ordered_address_ids.rbegin();
- const node_state &max_ns = _nodes.at(_address_vec[id_max]);
+ int id_max = *_ordered_host_port_ids.rbegin();
+ const node_state &max_ns = _nodes.at(host_port(_host_port_vec[id_max]));
Review Comment:
```suggestion
const node_state &max_ns = _nodes.at(_host_port_vec[id_max]);
```
--
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]
