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

[GitBox <gi...@apache.org>]

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