[GitHub] [hbase] Apache9 commented on a change in pull request #3678: HBASE-26251 StochasticLoadBalancer metrics should update even if bala…

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

Apache9 commented on a change in pull request #3678:
URL: https://github.com/apache/hbase/pull/3678#discussion_r708253151



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
##########
@@ -147,4 +147,17 @@
 
   /*Updates balancer status tag reported to JMX*/
   void updateBalancerStatus(boolean status);
+
+  /**
+   * In some scenarios, Balancer needs to update internal status or information according to the
+   * current tables load
+   * TODO: Maybe we can make a better name than 'updateBalancerLoadInfo'.
+   *
+   * @param loadOfAllTable region load of servers for all table
+   */
+  default void updateBalancerLoadInfo(Map<TableName, Map<ServerName, List<RegionInfo>>>
+    loadOfAllTable){}
+

Review comment:
       Remove these useless empty lines?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerChore.java
##########
@@ -46,7 +52,20 @@ public BalancerChore(HMaster master) {
   @Override
   protected void chore() {
     try {
-      master.balance();
+      BalanceResponse balanceResponse = master.balance();
+      //TODO: Now, only StochasticLoadBalancer has implemented the method
+      // 'updateBalancerLoadInfo'. Code here should be more generic if other balancers implement
+      // the method 'updateBalancerLoadInfo'
+      if (!balanceResponse.isBalancerRan()) {

Review comment:
       So the key change is here. When balancer is not run, we will get the current assignment snapshot and update the load info?




-- 
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: issues-unsubscribe@hbase.apache.org

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