cleanup

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java

@@ -778,11 +778,12 @@ public List<RegionPlan> doAction(BalanceAction action) {
         List<RegionPlan> mbRegionPlans = new ArrayList<>();
         for (int serverIndex : mba.getServerToRegionsToRemove().keySet()) {
           Set<Integer> regionsToRemove = mba.getServerToRegionsToRemove().get(serverIndex);
-          removeRegions(regionsPerServer[serverIndex], regionsToRemove);
+          regionsPerServer[serverIndex] =
+            removeRegions(regionsPerServer[serverIndex], regionsToRemove);
         }
         for (int serverIndex : mba.getServerToRegionsToAdd().keySet()) {
           Set<Integer> regionsToAdd = mba.getServerToRegionsToRemove().get(serverIndex);
-          addRegions(regionsPerServer[serverIndex], regionsToAdd);
+          regionsPerServer[serverIndex] = addRegions(regionsPerServer[serverIndex], regionsToAdd);
         }
         for (MoveRegionAction moveRegionAction : mba.getMoveActions()) {
           mbRegionPlans.add(regionMoved(moveRegionAction.getRegion(),
@@ -978,8 +979,8 @@ int[] removeRegions(int[] regions, Set<Integer> regionIndicesToRemove) {
 
     // If the newIndex is smaller than newSize, some regions were missing from the input array
     if (newIndex != newSize) {
-      throw new IllegalStateException(
-        "Region indices mismatch: some regions in the removal set were not found in the regions array");
+      throw new IllegalStateException("Region indices mismatch: some regions in the removal "
+        + "set were not found in the regions array");
     }
 
     return newRegions;

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DistributeReplicasCandidateGenerator.java

@@ -32,7 +32,7 @@
  * CandidateGenerator to distribute colocated replicas across different servers.
  */
 @InterfaceAudience.Private
-class DistributeReplicasCandidateGenerator extends RegionPlanConditionalCandidateGenerator {
+final class DistributeReplicasCandidateGenerator extends RegionPlanConditionalCandidateGenerator {
 
   static DistributeReplicasCandidateGenerator INSTANCE = new DistributeReplicasCandidateGenerator();
 

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DistributeReplicasConditional.java

@@ -23,6 +23,7 @@
 import java.util.Set;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.RegionReplicaUtil;
 import org.apache.hadoop.hbase.master.RegionPlan;
 import org.apache.hadoop.hbase.util.Pair;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -109,8 +110,8 @@ private boolean checkViolation(ReplicaKey movingReplicaKey, Set<RegionInfo> dest
 
   /**
    * This is necessary because it would be too expensive to use
-   * {@link org.apache.hadoop.hbase.client.RegionReplicaUtil#isReplicasForSameRegion(RegionInfo, RegionInfo)}
-   * for every combo of regions.
+   * {@link RegionReplicaUtil#isReplicasForSameRegion(RegionInfo, RegionInfo)} for every combo of
+   * regions.
    */
   static class ReplicaKey {
     private final Pair<ByteArrayWrapper, ByteArrayWrapper> startAndStopKeys;
@@ -122,8 +123,12 @@ static class ReplicaKey {
 
     @Override
     public boolean equals(Object o) {
-      if (this == o) return true;
-      if (!(o instanceof ReplicaKey)) return false;
+      if (this == o) {
+        return true;
+      }
+      if (!(o instanceof ReplicaKey)) {
+        return false;
+      }
       ReplicaKey other = (ReplicaKey) o;
       return this.startAndStopKeys.equals(other.startAndStopKeys);
     }

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java

@@ -91,7 +91,8 @@ public void setFavoredNodesManager(FavoredNodesManager fnm) {
   /** Returns any candidate generator in random */
   @Override
   protected CandidateGenerator getRandomGenerator(BalancerClusterState cluster) {
-    return candidateGenerators.get(ThreadLocalRandom.current().nextInt(candidateGenerators.size()));
+    return candidateGenerators.values().stream().toList()
+      .get(ThreadLocalRandom.current().nextInt(candidateGenerators.size()));
   }
 
   /**

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/MetaTableIsolationCandidateGenerator.java

@@ -21,7 +21,7 @@
 import org.apache.yetus.audience.InterfaceAudience;
 
 @InterfaceAudience.Private
-public class MetaTableIsolationCandidateGenerator extends TableIsolationCandidateGenerator {
+public final class MetaTableIsolationCandidateGenerator extends TableIsolationCandidateGenerator {
 
   static MetaTableIsolationCandidateGenerator INSTANCE = new MetaTableIsolationCandidateGenerator();
 

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/MoveBatchAction.java

@@ -18,10 +18,11 @@
 package org.apache.hadoop.hbase.master.balancer;
 
 import java.util.List;
-import org.apache.curator.shaded.com.google.common.collect.HashMultimap;
-import org.apache.curator.shaded.com.google.common.collect.Multimaps;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.common.collect.HashMultimap;
+import org.apache.hbase.thirdparty.com.google.common.collect.Multimaps;
+
 @InterfaceAudience.Private
 public class MoveBatchAction extends BalanceAction {
   private final List<MoveRegionAction> moveActions;

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/RegionPlanConditional.java

@@ -30,7 +30,7 @@
 
 @InterfaceAudience.Private
 @InterfaceStability.Evolving
-public abstract class RegionPlanConditional { // todo rename to BalancerConditional
+public abstract class RegionPlanConditional {
   private static final Logger LOG = LoggerFactory.getLogger(RegionPlanConditional.class);
   private BalancerClusterState cluster;
 

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java

@@ -160,15 +160,6 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
   protected Map<Class<? extends CandidateGenerator>, CandidateGenerator> candidateGenerators;
   private Map<Class<? extends CandidateGenerator>, Double> weightsOfGenerators;
 
-  public enum GeneratorType {
-    RANDOM,
-    LOAD,
-    LOCALITY,
-    RACK,
-    SYSTEM_TABLE_ISOLATION,
-    META_TABLE_ISOLATION,
-  }
-
   private final BalancerConditionals balancerConditionals = BalancerConditionals.INSTANCE;
 
   /**
@@ -466,20 +457,25 @@ protected CandidateGenerator getRandomGenerator(BalancerClusterState cluster) {
     }
 
     double rand = ThreadLocalRandom.current().nextDouble();
-    Class<? extends CandidateGenerator> greatestWeightClazz = null;
+    CandidateGenerator greatestWeightGenerator = null;
     double greatestWeight = 0;
-    for (Class<? extends CandidateGenerator> clazz : candidateGenerators.keySet()) {
+    for (Map.Entry<Class<? extends CandidateGenerator>,
+      CandidateGenerator> entry : candidateGenerators.entrySet()) {
+      Class<? extends CandidateGenerator> clazz = entry.getKey();
       double generatorWeight = weightsOfGenerators.get(clazz);
       if (generatorWeight > greatestWeight) {
         greatestWeight = generatorWeight;
-        greatestWeightClazz = clazz;
+        greatestWeightGenerator = entry.getValue();
       }
       if (rand <= generatorWeight) {
-        return candidateGenerators.get(clazz);
+        return entry.getValue();
       }
     }
 
-    return candidateGenerators.getOrDefault(greatestWeightClazz, new RandomCandidateGenerator());
+    if (greatestWeightGenerator != null) {
+      return greatestWeightGenerator;
+    }
+    return candidateGenerators.values().stream().findAny().orElseThrow();
   }
 
   @RestrictedApi(explanation = "Should only be called in tests", link = "",
@@ -497,6 +493,7 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
    * approach the optimal state given enough steps.
    */
   @Override
+  @SuppressWarnings("checkstyle:MethodLength")
   protected List<RegionPlan> balanceTable(TableName tableName,
     Map<ServerName, List<RegionInfo>> loadOfOneTable) {
     // On clusters with lots of HFileLinks or lots of reference files,
@@ -636,7 +633,7 @@ protected List<RegionPlan> balanceTable(TableName tableName,
     StringBuilder logMessage = new StringBuilder("CandidateGenerator activity summary:\n");
     generatorToStepCount.forEach((generator, count) -> {
       long approvals = generatorToApprovedActionCount.getOrDefault(generator, 0L);
-      logMessage.append(String.format(" - %s: %d steps, %d approvals\n", generator.getSimpleName(),
+      logMessage.append(String.format(" - %s: %d steps, %d approvals%n", generator.getSimpleName(),
         count, approvals));
     });
     // Log the message

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/SystemTableIsolationCandidateGenerator.java

@@ -21,7 +21,7 @@
 import org.apache.yetus.audience.InterfaceAudience;
 
 @InterfaceAudience.Private
-public class SystemTableIsolationCandidateGenerator extends TableIsolationCandidateGenerator {
+public final class SystemTableIsolationCandidateGenerator extends TableIsolationCandidateGenerator {
 
   static SystemTableIsolationCandidateGenerator INSTANCE =
     new SystemTableIsolationCandidateGenerator();

hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/CandidateGeneratorTestUtil.java

@@ -43,6 +43,9 @@ public final class CandidateGeneratorTestUtil {
 
   private static final Logger LOG = LoggerFactory.getLogger(CandidateGeneratorTestUtil.class);
 
+  private CandidateGeneratorTestUtil() {
+  }
+
   static void runBalancerToExhaustion(Configuration conf,
     Map<ServerName, List<RegionInfo>> serverToRegions,
     Set<Function<BalancerClusterState, Boolean>> expectations) {
@@ -144,9 +147,8 @@ static void printClusterDistribution(BalancerClusterState cluster, long actionsT
   }
 
   /**
-   * Partitions the given serverToRegions map by table, returning a structure of Map<TableName,
-   * Map<ServerName, List<RegionInfo>>>. The tables are derived from the RegionInfo objects found in
-   * serverToRegions.
+   * Partitions the given serverToRegions map by table The tables are derived from the RegionInfo
+   * objects found in serverToRegions.
    * @param serverToRegions The map of servers to their assigned regions.
    * @return A map of tables to their server-to-region assignments.
    */

hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestLargeClusterBalancingMultiTableIsolationReplicaDistribution.java

@@ -20,7 +20,11 @@
 import static org.apache.hadoop.hbase.master.balancer.CandidateGeneratorTestUtil.isTableIsolated;
 import static org.apache.hadoop.hbase.master.balancer.CandidateGeneratorTestUtil.runBalancerToExhaustion;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.ServerName;

hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestLargeClusterBalancingReplicaDistribution.java

@@ -19,7 +19,11 @@
 
 import static org.apache.hadoop.hbase.master.balancer.CandidateGeneratorTestUtil.runBalancerToExhaustion;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.ServerName;