saintstack commented on a change in pull request #3248:
URL: https://github.com/apache/hbase/pull/3248#discussion_r629719036
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
##########
@@ -59,10 +59,11 @@
*/
@Deprecated
String HBASE_RSGROUP_LOADBALANCER_CLASS =
"hbase.rsgroup.grouploadbalancer.class";
+
/**
* Set the current cluster status. This allows a LoadBalancer to map host
name to a server
*/
- void setClusterMetrics(ClusterMetrics st);
+ void updateClusterMetrics(ClusterMetrics metrics);
Review comment:
We change the method name because the method is now being used
differently? Before we called 'set' once? But now we 'update' continuously
during operation? Or was it just always incorrectly named?
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
##########
@@ -53,6 +52,10 @@
* The base class for load balancers. It provides the the functions used to by
* {@code AssignmentManager} to assign regions in the edge cases. It doesn't
provide an
* implementation of the actual balancing algorithm.
+ * <p/>
+ * Since 3.0.0, all the balancers will be wrapped inside a {@code
RSGroupBasedLoadBalancer}, it will
+ * be in charge of the synchronization of balancing and configuration
changing, so we do not need to
+ * synchronized by ourselves.
Review comment:
Interesting. Wondering what the rationale for this (Perhaps it later in
this patch)
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -400,6 +387,12 @@ BalanceAction nextAction(BalancerClusterState cluster) {
.generate(cluster);
}
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
Review comment:
Instead of @VisibleForTests? (I like this annotation)
--
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:
[email protected]
