You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/08 14:47:19 UTC

[GitHub] [hbase] virajjasani commented on a change in pull request #1926: HBASE-24586 Add table level locality in table.jsp

virajjasani commented on a change in pull request #1926:
URL: https://github.com/apache/hbase/pull/1926#discussion_r451599800



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -228,33 +228,49 @@ public long getUniqueBlocksTotalWeight() {
    * Implementations 'visit' hostAndWeight.
    */
   public interface Visitor {
-    float visit(final HostAndWeight hostAndWeight);
+    long visit(final HostAndWeight hostAndWeight);
   }
 
   /**
    * @param host the host name
    * @return the locality index of the given host
    */
   public float getBlockLocalityIndex(String host) {
-    return getBlockLocalityIndexInternal(host,
-      e -> (float) e.weight / (float) uniqueBlocksTotalWeight);
+    return (float) getBlocksLocalityWeightInternal(host, e -> e.weight)
+      / (float) uniqueBlocksTotalWeight;
   }
 
   /**
    * @param host the host name
    * @return the locality index with ssd of the given host
    */
   public float getBlockLocalityIndexForSsd(String host) {
-    return getBlockLocalityIndexInternal(host,
-      e -> (float) e.weightForSsd / (float) uniqueBlocksTotalWeight);
+    return (float) getBlocksLocalityWeightInternal(host, e -> e.weightForSsd)
+      / (float) uniqueBlocksTotalWeight;
+  }
+
+  /**
+   * @param host the host name
+   * @return the blocks local weight of the given host
+   */
+  public long getBlocksLocalWeight(String host) {
+    return getBlocksLocalityWeightInternal(host, e -> e.weight);

Review comment:
       same here

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -228,33 +228,49 @@ public long getUniqueBlocksTotalWeight() {
    * Implementations 'visit' hostAndWeight.
    */
   public interface Visitor {
-    float visit(final HostAndWeight hostAndWeight);
+    long visit(final HostAndWeight hostAndWeight);
   }
 
   /**
    * @param host the host name
    * @return the locality index of the given host
    */
   public float getBlockLocalityIndex(String host) {
-    return getBlockLocalityIndexInternal(host,
-      e -> (float) e.weight / (float) uniqueBlocksTotalWeight);
+    return (float) getBlocksLocalityWeightInternal(host, e -> e.weight)
+      / (float) uniqueBlocksTotalWeight;
   }
 
   /**
    * @param host the host name
    * @return the locality index with ssd of the given host
    */
   public float getBlockLocalityIndexForSsd(String host) {
-    return getBlockLocalityIndexInternal(host,
-      e -> (float) e.weightForSsd / (float) uniqueBlocksTotalWeight);
+    return (float) getBlocksLocalityWeightInternal(host, e -> e.weightForSsd)

Review comment:
       same here: `getBlocksLocalityWeightInternal(host, HostAndWeight::getWeightForSsd)`

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -228,33 +228,49 @@ public long getUniqueBlocksTotalWeight() {
    * Implementations 'visit' hostAndWeight.
    */
   public interface Visitor {
-    float visit(final HostAndWeight hostAndWeight);
+    long visit(final HostAndWeight hostAndWeight);
   }
 
   /**
    * @param host the host name
    * @return the locality index of the given host
    */
   public float getBlockLocalityIndex(String host) {
-    return getBlockLocalityIndexInternal(host,
-      e -> (float) e.weight / (float) uniqueBlocksTotalWeight);
+    return (float) getBlocksLocalityWeightInternal(host, e -> e.weight)

Review comment:
       Can we use method reference here: `getBlocksLocalityWeightInternal(host, HostAndWeight::getWeight)` ?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -228,33 +228,49 @@ public long getUniqueBlocksTotalWeight() {
    * Implementations 'visit' hostAndWeight.
    */
   public interface Visitor {
-    float visit(final HostAndWeight hostAndWeight);
+    long visit(final HostAndWeight hostAndWeight);
   }
 
   /**
    * @param host the host name
    * @return the locality index of the given host
    */
   public float getBlockLocalityIndex(String host) {
-    return getBlockLocalityIndexInternal(host,
-      e -> (float) e.weight / (float) uniqueBlocksTotalWeight);
+    return (float) getBlocksLocalityWeightInternal(host, e -> e.weight)
+      / (float) uniqueBlocksTotalWeight;
   }
 
   /**
    * @param host the host name
    * @return the locality index with ssd of the given host
    */
   public float getBlockLocalityIndexForSsd(String host) {
-    return getBlockLocalityIndexInternal(host,
-      e -> (float) e.weightForSsd / (float) uniqueBlocksTotalWeight);
+    return (float) getBlocksLocalityWeightInternal(host, e -> e.weightForSsd)
+      / (float) uniqueBlocksTotalWeight;
+  }
+
+  /**
+   * @param host the host name
+   * @return the blocks local weight of the given host
+   */
+  public long getBlocksLocalWeight(String host) {
+    return getBlocksLocalityWeightInternal(host, e -> e.weight);
+  }
+
+  /**
+   * @param host the host name
+   * @return the blocks local with ssd weight of the given host
+   */
+  public long getBlocksLocalWithSsdWeight(String host) {
+    return getBlocksLocalityWeightInternal(host, e -> e.weightForSsd);

Review comment:
       same here

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -228,33 +228,49 @@ public long getUniqueBlocksTotalWeight() {
    * Implementations 'visit' hostAndWeight.
    */
   public interface Visitor {
-    float visit(final HostAndWeight hostAndWeight);
+    long visit(final HostAndWeight hostAndWeight);

Review comment:
       Can we not continue with `float`? Or is it that all instance var of `HostAndWeight` are long and hence, we want them as long here?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -228,33 +228,49 @@ public long getUniqueBlocksTotalWeight() {
    * Implementations 'visit' hostAndWeight.

Review comment:
       nit: `HostAndWeight` has `host` variable which is just set by constructor. We can make it final with this PR.




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

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