aQuigs commented on code in PR #7123:
URL: https://github.com/apache/hadoop/pull/7123#discussion_r1828553763


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##########
@@ -4738,6 +4738,12 @@ public long getMissingReplOneBlocksCount() {
     // not locking
     return blockManager.getMissingReplOneBlocksCount();
   }
+
+  @Metric({"BadlyDistBlocks", "Number of Badly Distributed Blocks"})
+  public long getBadlyDistributedBlocksCount() {
+    // not locking
+    return blockManager.getBadlyDistributedBlocksCount();
+  }

Review Comment:
   This metric looks like a duplicate of below. Is that intentional?



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/ECBlockGroupStats.java:
##########
@@ -99,7 +107,8 @@ public String toString() {
         .append(", BytesInFutureBlockGroups=").append(
             getBytesInFutureBlockGroups())
         .append(", PendingDeletionBlocks=").append(
-            getPendingDeletionBlocks());
+            getPendingDeletionBlocks())
+        .append(" , 
badlyDistributedBlocks=").append(getBadlyDistributedBlocks());

Review Comment:
   ```suggestion
           .append(" , 
BadlyDistributedBlocks=").append(getBadlyDistributedBlocks());
   ```



##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java:
##########
@@ -5186,6 +5191,11 @@ public long getMissingReplOneBlocksCount() {
     return this.neededReconstruction.getCorruptReplicationOneBlockSize();
   }
 
+  public long getBadlyDistributedBlocksCount() {
+    // not locking
+    return this.neededReconstruction.getBadlyDistributedBlocks();
+  }
+

Review Comment:
   Looks redundant with `getBadlyDistributedBlocks()`?



##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/metrics/ECBlockGroupsMBean.java:
##########
@@ -58,7 +58,7 @@ public interface ECBlockGroupsMBean {
   long getPendingDeletionECBlocks();
 
   /**
-   * Return total number of erasure coded block groups.
+   * @return total number of erasure coded block groups.

Review Comment:
   Nit - this is the only change in this file and all the methods above have 
the old syntax too. Maybe we should scope this for a different PR to keep the 
diff clean (and address the entire class)



-- 
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]

Reply via email to