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

[GitBox]

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

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

[GitBox]

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



##
File path: hbase-protocol-shaded/src/main/protobuf/server/ClusterStatus.proto
##
@@ -133,8 +133,8 @@ message RegionLoad {
   /** the most recent sequence Id from cache flush */
   optional uint64 complete_sequence_id = 15;
 
-  /** The current data locality for region in the regionserver */
-  optional float data_locality = 16;
+  /** The current blocks local weight for region in the regionserver */
+  optional uint64 blocks_local_weight = 16;

Review comment:
   Same applies to ClusterStatus. Can we just add `float 
data_locality_for_ssd` at the end (24th element) and keep `data_locality` and 
`data_locality_for_ssd` as is?

##
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/RegionMetrics.java
##
@@ -141,9 +141,9 @@ default String getNameAsString() {
   Size getUncompressedStoreFileSize();
 
   /**
-   * @return the data locality of region in the regionserver.
+   * @return the data at local weight of this region in the regionserver
*/
-  float getDataLocality();
+  long getBlocksLocalWeight();

Review comment:
   RegionMetrics is IA.Public, if we are intending this patch to go to 
branch-2 and branch-1, we should better not modify existing methods, only add 
new ones.





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:
us...@infra.apache.org