keith-turner commented on code in PR #5070:
URL: https://github.com/apache/accumulo/pull/5070#discussion_r1844265835


##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -1057,11 +1059,29 @@ private long balanceTablets() {
         int attemptNum = 0;
         do {
           log.debug("Balancing for tables at level {}, times-in-loop: {}", dl, 
++attemptNum);
-          params = BalanceParamsImpl.fromThrift(tserverStatusForBalancerLevel,
-              tserverStatusForLevel, partitionedMigrations.get(dl));
+
+          SortedMap<TabletServerId,TServerStatus> statusForBalancerLevel =
+              tserverStatusForBalancerLevel;
+          if (attemptNum % checkInterval == 0) {

Review Comment:
   > I think the HostRegexTableLoadBalancer will do this 
[here](https://github.com/apache/accumulo/blob/2.1/core/src/main/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancer.java#L400).
   
   Ok and following that tablet id map it gets it from 
[here](https://github.com/apache/accumulo/blob/5715f06fc8eba911c89ffa86ee12dff1a64127aa/core/src/main/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancer.java#L382)
 which is coming from this 
[balancerEnv](https://github.com/apache/accumulo/blob/5715f06fc8eba911c89ffa86ee12dff1a64127aa/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java#L423)
 that is only initialized once for the manager process.  So when the manager is 
intending to only balance the root table its not restricting that table id map, 
so the balancer is completely unaware of this outer goal.
   
   This is making me wonder if we should back out all of the changes for 
balancing by level in 2.1 for now and try to figure out how to accomplish that 
goal in a better way.  As it stands now the manager code has a goal for only 
balancing the root or metadata table and the balancer plugin is completely 
unaware of that goal.  Going down the path of making the balancer aware of 
those goals may introduce more bugs in 2.1.  That is why I am wondering if we 
should back out the changes for now and try to figure out how to balance by 
level in follow on work



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

Reply via email to