Fix invalid metadata rate calculation

Fixed the calculation of invalid metadata rate in RebalanceUtils. Also added more/different pretty printing of a rebalance plan.

src/java/voldemort/client/rebalance/RebalancePlan.java

@@ -115,11 +115,6 @@ public RebalancePlan(final Cluster currentCluster,
         logger.info("Final cluster : " + finalCluster);
         logger.info("Batch size : " + batchSize);
 
-        logger.info(RebalanceUtils.analyzeInvalidMetadataRate(currentCluster,
-                                                              currentStoreDefs,
-                                                              finalCluster,
-                                                              finalStoreDefs));
-
         // Initialize the plan
         batchPlans = new ArrayList<RebalanceBatchPlan>();
 
@@ -291,6 +286,19 @@ public List<RebalanceBatchPlan> getPlan() {
         return batchPlans;
     }
 
+    /**
+     * Total number of rebalancing tasks in the plan.
+     * 
+     * @return number of rebalancing tasks in the plan.
+     */
+    public int taskCount() {
+        int numTasks = 0;
+        for(RebalanceBatchPlan batchPlan: batchPlans) {
+            numTasks += batchPlan.getBatchPlan().size();
+        }
+        return numTasks;
+    }
+
     public int getPrimariesMoved() {
         return numPrimaryPartitionMoves;
     }
@@ -314,12 +322,19 @@ public MoveMap getZoneMoveMap() {
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();
-        // Dump entire plan batch-by-batch, partition info-by-partition info...
+        // Add entire plan batch-by-batch, partition info-by-partition info...
         for(RebalanceBatchPlan batchPlan: batchPlans) {
             sb.append(batchPlan).append(Utils.NEWLINE);
         }
-        // Dump aggregate stats of the plan
+        // Add invalid metadata rate analysis
+        sb.append(RebalanceUtils.analyzeInvalidMetadataRate(currentCluster,
+                                                            currentStoreDefs,
+                                                            finalCluster,
+                                                            finalStoreDefs));
+        // Summarize aggregate plan stats
         sb.append("Total number of primary partition moves : " + numPrimaryPartitionMoves)
+          .append(Utils.NEWLINE)
+          .append("Total number of rebalance tasks : " + taskCount())
           .append(Utils.NEWLINE)
           .append("Total number of partition-store moves : " + numPartitionStoreMoves)
           .append(Utils.NEWLINE)

src/java/voldemort/client/rebalance/task/StealerBasedRebalanceTask.java

@@ -41,7 +41,8 @@ public class StealerBasedRebalanceTask extends RebalanceTask {
     // submits "work" to the server and servers are mature enough to throttle
     // and process them as fast as they can. Since that looks like changing all
     // the server execution frameworks, let's stick with this for now..
-    // TODO: Talk with Lei and Vinoth and decide on the fate of ma
+    // TODO: Decide fate of maxTries argument after some integration tests are
+    // done.
     private final int maxTries;
 
     public StealerBasedRebalanceTask(final int taskId,

src/java/voldemort/utils/RebalanceUtils.java

@@ -1104,25 +1104,27 @@ public Thread newThread(Runnable r) {
         });
     }
 
+    // TODO: (refactor) separate analysis from pretty printing and add a unit
+    // test for the analysis sub-method.
     /**
-     * Compares current cluster with target cluster. Uses pertinent store defs
+     * Compares current cluster with final cluster. Uses pertinent store defs
      * for each cluster to determine if a node that hosts a zone-primary in the
-     * current cluster will no longer host any zone-nary in the target cluster.
+     * current cluster will no longer host any zone-nary in the final cluster.
      * This check is the precondition for a server returning an invalid metadata
      * exception to a client on a normal-case put or get. Normal-case being that
      * the zone-primary receives the pseudo-master put or the get operation.
      * 
      * @param currentCluster
      * @param currentStoreDefs
-     * @param targetCluster
-     * @param targetStoreDefs
+     * @param finalCluster
+     * @param finalStoreDefs
      * @return pretty-printed string documenting invalid metadata rates for each
      *         zone.
      */
     public static String analyzeInvalidMetadataRate(final Cluster currentCluster,
                                                     List<StoreDefinition> currentStoreDefs,
-                                                    final Cluster targetCluster,
-                                                    List<StoreDefinition> targetStoreDefs) {
+                                                    final Cluster finalCluster,
+                                                    List<StoreDefinition> finalStoreDefs) {
         StringBuilder sb = new StringBuilder();
         sb.append("Dump of invalid metadata rates per zone").append(Utils.NEWLINE);
 
@@ -1135,26 +1137,32 @@ public static String analyzeInvalidMetadataRate(final Cluster currentCluster,
               .append(Utils.NEWLINE);
 
             StoreRoutingPlan currentSRP = new StoreRoutingPlan(currentCluster, currentStoreDef);
-            StoreDefinition targetStoreDef = StoreUtils.getStoreDef(targetStoreDefs,
-                                                                    currentStoreDef.getName());
-            StoreRoutingPlan targetSRP = new StoreRoutingPlan(targetCluster, targetStoreDef);
+            StoreDefinition finalStoreDef = StoreUtils.getStoreDef(finalStoreDefs,
+                                                                   currentStoreDef.getName());
+            StoreRoutingPlan finalSRP = new StoreRoutingPlan(finalCluster, finalStoreDef);
 
             // Only care about existing zones
             for(int zoneId: currentCluster.getZoneIds()) {
-                int zoneLocalPrimaries = 0;
+                int zonePrimariesCount = 0;
                 int invalidMetadata = 0;
+
                 // Examine nodes in current cluster in existing zone.
                 for(int nodeId: currentCluster.getNodeIdsInZone(zoneId)) {
-                    for(int partitionId: targetSRP.getZonePrimaryPartitionIds(nodeId)) {
-                        zoneLocalPrimaries++;
-                        if(!currentSRP.getZoneNAryPartitionIds(nodeId).contains(partitionId)) {
+                    // For every zone-primary in current cluster
+                    for(int zonePrimaryPartitionId: currentSRP.getZonePrimaryPartitionIds(nodeId)) {
+                        zonePrimariesCount++;
+                        // Determine if original zone-primary node is still some
+                        // form of n-ary in final cluster. If not,
+                        // InvalidMetadataException will fire.
+                        if(!finalSRP.getZoneNAryPartitionIds(nodeId)
+                                    .contains(zonePrimaryPartitionId)) {
                             invalidMetadata++;
                         }
                     }
                 }
-                float rate = invalidMetadata / (float) zoneLocalPrimaries;
+                float rate = invalidMetadata / (float) zonePrimariesCount;
                 sb.append("\tZone " + zoneId)
-                  .append(" : total zone primaries " + zoneLocalPrimaries)
+                  .append(" : total zone primaries " + zonePrimariesCount)
                   .append(", # that trigger invalid metadata " + invalidMetadata)
                   .append(" => " + rate)
                   .append(Utils.NEWLINE);