Re: [PR] refactor: Simplify code on partition_configuration [incubator-pegasus]

["empiredan (via GitHub)" <gi...@apache.org>]

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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org