vaijosh commented on code in PR #7900:
URL: https://github.com/apache/hbase/pull/7900#discussion_r2915944433
##########
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/CacheAwareLoadBalancer.java:
##########
@@ -173,6 +173,11 @@ private RegionInfo
getRegionInfoByEncodedName(BalancerClusterState cluster, Stri
@Override
public void throttle(RegionPlan plan) {
+ throttle(plan, this);
+ }
+
+ @Override
+ public void throttle(RegionPlan plan, Object syncMonitor) {
Review Comment:
@hingu-8103
This implementation introduces tight coupling.
Currently, RSGroupBasedLoadBalancer acquires the monitor, but
CacheAwareLoadBalancer is responsible for releasing it via wait(). This makes
the synchronization flow very difficult to trace and maintain.
Can we refactor CacheAwareLoadBalancer.throttle() to simply return a long
(the required sleep time)? This allows the RSGroupBasedLoadBalancer to manage
its own monitor and call this.wait(delay) directly. This keeps the locking
logic encapsulated in one place.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
