empiredan commented on code in PR #2051:
URL:
https://github.com/apache/incubator-pegasus/pull/2051#discussion_r1677672770
##########
src/meta/meta_data.h:
##########
@@ -510,29 +504,30 @@ inline health_status partition_health_status(const
partition_configuration &pc,
int
mutation_2pc_min_replica_count)
{
if (!pc.hp_primary) {
- if (pc.hp_secondaries.empty())
+ if (pc.hp_secondaries.empty()) {
return HS_DEAD;
- else
+ } else {
return HS_UNREADABLE;
+ }
} else {
Review Comment:
`else` could be dropped.
##########
src/meta/test/update_configuration_test.cpp:
##########
@@ -394,8 +395,9 @@ static void clone_app_mapper(app_mapper &output, const
app_mapper &input)
const std::shared_ptr<app_state> &old_app = iter.second;
dsn::app_info info = *old_app;
std::shared_ptr<app_state> new_app = app_state::create(info);
- for (unsigned int i = 0; i != old_app->partition_count; ++i)
- new_app->partitions[i] = old_app->partitions[i];
+ for (unsigned int i = 0; i != old_app->partition_count; ++i) {
+ new_app->pcs[i] = old_app->pcs[i];
+ }
Review Comment:
```suggestion
new_app->pcs = old_app->pcs;
```
##########
src/shell/commands/table_management.cpp:
##########
@@ -437,9 +430,9 @@ bool app_disk(command_executor *e, shell_context *sc,
arguments args)
}
if (detailed) {
- tp_details.add_row(std::to_string(p.pid.get_partition_index()));
- tp_details.append_data(p.ballot);
- tp_details.append_data(replica_count_str);
+ tp_details.add_row(std::to_string(pc.pid.get_partition_index()));
+ tp_details.append_data(pc.ballot);
+ tp_details.append_data(fmt::format("{}/{}", replica_count,
pc.max_replica_count));
Review Comment:
And `replica_count` could be dropped:
```suggestion
tp_details.append_data(fmt::format("{}/{}",
pc.hp_secondaries.size() + (pc.hp_primary ? 1 : 0), pc.max_replica_count));
```
##########
src/server/available_detector.cpp:
##########
@@ -266,8 +266,7 @@ void available_detector::report_availability_info()
bool available_detector::generate_hash_keys()
{
// get app_id and partition_count.
- auto err =
- _ddl_client->list_app(FLAGS_available_detect_app, _app_id,
_partition_count, partitions);
+ auto err = _ddl_client->list_app(FLAGS_available_detect_app, _app_id,
_partition_count, pcs);
Review Comment:
Could `pcs` be declared as a local variable ?
##########
src/meta/meta_data.h:
##########
@@ -510,29 +504,30 @@ inline health_status partition_health_status(const
partition_configuration &pc,
int
mutation_2pc_min_replica_count)
{
if (!pc.hp_primary) {
- if (pc.hp_secondaries.empty())
+ if (pc.hp_secondaries.empty()) {
return HS_DEAD;
- else
+ } else {
return HS_UNREADABLE;
+ }
Review Comment:
```suggestion
}
return HS_UNREADABLE;
```
--
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]
