HBASE-25834 Remove balanceTable method from LoadBalancer interface (#3217)
Signed-off-by: Yulin Niu <niuyulin@apache.org>
diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
index 491b5fa..a6f5357 100644
--- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
+++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
@@ -72,25 +72,13 @@
void setClusterInfoProvider(ClusterInfoProvider provider);
/**
- * Perform the major balance operation for cluster, will invoke {@link #balanceTable} to do actual
- * balance. Normally not need override this method, except {@link SimpleLoadBalancer} and
- * {@code RSGroupBasedLoadBalancer}
+ * Perform the major balance operation for cluster.
* @param loadOfAllTable region load of servers for all table
* @return a list of regions to be moved, including source and destination, or null if cluster is
* already balanced
*/
- List<RegionPlan> balanceCluster(Map<TableName,
- Map<ServerName, List<RegionInfo>>> loadOfAllTable) throws IOException;
-
- /**
- * Perform the major balance operation for table, all class implement of {@link LoadBalancer}
- * should override this method
- * @param tableName the table to be balanced
- * @param loadOfOneTable region load of servers for the specific one table
- * @return List of plans
- */
- List<RegionPlan> balanceTable(TableName tableName,
- Map<ServerName, List<RegionInfo>> loadOfOneTable);
+ List<RegionPlan> balanceCluster(Map<TableName, Map<ServerName, List<RegionInfo>>> loadOfAllTable)
+ throws IOException;
/**
* Perform a Round Robin assignment of regions.
diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
index 56fb96e..93a7331 100644
--- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
+++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
@@ -610,13 +610,44 @@
return returnMap;
}
- @Override
- public abstract List<RegionPlan> balanceTable(TableName tableName,
- Map<ServerName, List<RegionInfo>> loadOfOneTable);
+ /**
+ * Perform the major balance operation for table, all sub classes should override this method.
+ * <p/>
+ * Will be invoked by {@link #balanceCluster(Map)}. If
+ * {@link HConstants#HBASE_MASTER_LOADBALANCE_BYTABLE} is enabled, we will call this method
+ * multiple times, one table a time, where we will only pass in the regions for a single table
+ * each time. If not, we will pass in all the regions at once, and the {@code tableName} will be
+ * {@link HConstants#ENSEMBLE_TABLE_NAME}.
+ * @param tableName the table to be balanced
+ * @param loadOfOneTable region load of servers for the specific one table
+ * @return List of plans
+ */
+ protected abstract List<RegionPlan> balanceTable(TableName tableName,
+ Map<ServerName, List<RegionInfo>> loadOfOneTable);
+ /**
+ * Called before actually executing balanceCluster. The sub classes could override this method to
+ * do some initialization work.
+ */
+ protected void
+ preBalanceCluster(Map<TableName, Map<ServerName, List<RegionInfo>>> loadOfAllTable) {
+ }
+
+ /**
+ * Perform the major balance operation for cluster, will invoke
+ * {@link #balanceTable(TableName, Map)} to do actual balance.
+ * <p/>
+ * THIs method is marked as final which means you should not override this method. See the javadoc
+ * for {@link #balanceTable(TableName, Map)} for more details.
+ * @param loadOfAllTable region load of servers for all table
+ * @return a list of regions to be moved, including source and destination, or null if cluster is
+ * already balanced
+ * @see #balanceTable(TableName, Map)
+ */
@Override
- public List<RegionPlan>
- balanceCluster(Map<TableName, Map<ServerName, List<RegionInfo>>> loadOfAllTable) {
+ public final synchronized List<RegionPlan>
+ balanceCluster(Map<TableName, Map<ServerName, List<RegionInfo>>> loadOfAllTable) {
+ preBalanceCluster(loadOfAllTable);
if (isByTable) {
List<RegionPlan> result = new ArrayList<>();
loadOfAllTable.forEach((tableName, loadOfOneTable) -> {
diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java
index a8b161a..57fae11 100644
--- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java
+++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java
@@ -123,6 +123,13 @@
}
@Override
+ protected void
+ preBalanceCluster(Map<TableName, Map<ServerName, List<RegionInfo>>> loadOfAllTable) {
+ // We need clusterLoad of all regions on every server to achieve overall balanced
+ setClusterLoad(loadOfAllTable);
+ }
+
+ @Override
public void onConfigurationChange(Configuration conf) {
float originSlop = slop;
float originOverallSlop = overallSlop;
@@ -247,7 +254,7 @@
* or null if cluster is already balanced
*/
@Override
- public List<RegionPlan> balanceTable(TableName tableName,
+ protected List<RegionPlan> balanceTable(TableName tableName,
Map<ServerName, List<RegionInfo>> loadOfOneTable) {
long startTime = System.currentTimeMillis();
@@ -466,7 +473,7 @@
* max. Together with other regions left to be assigned, we distribute all regionToMove, to the RS
* that have less regions in whole cluster scope.
*/
- public void balanceOverall(List<RegionPlan> regionsToReturn,
+ private void balanceOverall(List<RegionPlan> regionsToReturn,
Map<ServerName, BalanceInfo> serverBalanceInfo, boolean fetchFromTail,
MinMaxPriorityQueue<RegionPlan> regionsToMove, int max, int min) {
// Step 1.
@@ -613,15 +620,4 @@
rp.setDestination(sn);
regionsToReturn.add(rp);
}
-
- /**
- * Override to invoke {@link #setClusterLoad} before balance, We need clusterLoad of all regions
- * on every server to achieve overall balanced
- */
- @Override
- public synchronized List<RegionPlan>
- balanceCluster(Map<TableName, Map<ServerName, List<RegionInfo>>> loadOfAllTable) {
- setClusterLoad(loadOfAllTable);
- return super.balanceCluster(loadOfAllTable);
- }
}
diff --git a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
index 2e33cf1..a6ae2ac 100644
--- a/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
+++ b/hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
@@ -112,14 +112,9 @@
}
public static class MockBalancer extends BaseLoadBalancer {
- @Override
- public List<RegionPlan>
- balanceCluster(Map<TableName, Map<ServerName, List<RegionInfo>>> loadOfAllTable) {
- return null;
- }
@Override
- public List<RegionPlan> balanceTable(TableName tableName,
+ protected List<RegionPlan> balanceTable(TableName tableName,
Map<ServerName, List<RegionInfo>> loadOfOneTable) {
return null;
}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java
index aa3642a..17b2863 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java
@@ -94,7 +94,7 @@
}
@Override
- public List<RegionPlan> balanceTable(TableName tableName,
+ protected List<RegionPlan> balanceTable(TableName tableName,
Map<ServerName, List<RegionInfo>> loadOfOneTable) {
// TODO. Look at is whether Stochastic loadbalancer can be integrated with this
List<RegionPlan> plans = new ArrayList<>();
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java
index 4cb873e..742863d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java
@@ -665,7 +665,7 @@
* implementation. For the misplaced regions, we assign a bogus server to it and AM takes care.
*/
@Override
- public List<RegionPlan> balanceTable(TableName tableName,
+ protected List<RegionPlan> balanceTable(TableName tableName,
Map<ServerName, List<RegionInfo>> loadOfOneTable) {
if (this.services != null) {
List<RegionPlan> regionPlans = Lists.newArrayList();
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java
index d009e3b..11a2102 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java
@@ -66,12 +66,6 @@
return Collections.emptyList();
}
- @Override
- public List<RegionPlan> balanceTable(TableName tableName,
- Map<ServerName, List<RegionInfo>> loadOfOneTable) {
- return Collections.emptyList();
- }
-
private Map<ServerName, List<RegionInfo>> assign(Collection<RegionInfo> regions,
List<ServerName> servers) {
// should only have 1 region server in maintenance mode
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
index 0ffdc43..9a7823d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
@@ -405,7 +405,7 @@
* should always approach the optimal state given enough steps.
*/
@Override
- public synchronized List<RegionPlan> balanceTable(TableName tableName, Map<ServerName,
+ protected List<RegionPlan> balanceTable(TableName tableName, Map<ServerName,
List<RegionInfo>> loadOfOneTable) {
// On clusters with lots of HFileLinks or lots of reference files,
// instantiating the storefile infos can be quite expensive.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
index bedc776..fe82e25 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
@@ -123,8 +123,7 @@
}
/**
- * Override to balance by RSGroup
- * not invoke {@link #balanceTable(TableName, Map)}
+ * Balance by RSGroup.
*/
@Override
public List<RegionPlan> balanceCluster(
@@ -449,40 +448,6 @@
internalBalancer.updateBalancerStatus(status);
}
- /**
- * can achieve table balanced rather than overall balanced
- */
- @Override
- public List<RegionPlan> balanceTable(TableName tableName,
- Map<ServerName, List<RegionInfo>> loadOfOneTable) {
- if (!isOnline()) {
- LOG.error(RSGroupInfoManager.class.getSimpleName()
- + " is not online, unable to perform balanceTable");
- return null;
- }
- Map<TableName, Map<ServerName, List<RegionInfo>>> loadOfThisTable = new HashMap<>();
- loadOfThisTable.put(tableName, loadOfOneTable);
- Pair<Map<TableName, Map<ServerName, List<RegionInfo>>>, List<RegionPlan>>
- correctedStateAndRegionPlans;
- // Calculate correct assignments and a list of RegionPlan for mis-placed regions
- try {
- correctedStateAndRegionPlans = correctAssignments(loadOfThisTable);
- } catch (IOException e) {
- LOG.error("get correct assignments and mis-placed regions error ", e);
- return null;
- }
- Map<TableName, Map<ServerName, List<RegionInfo>>> correctedLoadOfThisTable =
- correctedStateAndRegionPlans.getFirst();
- List<RegionPlan> regionPlans = correctedStateAndRegionPlans.getSecond();
- List<RegionPlan> tablePlans =
- this.internalBalancer.balanceTable(tableName, correctedLoadOfThisTable.get(tableName));
-
- if (tablePlans != null) {
- regionPlans.addAll(tablePlans);
- }
- return regionPlans;
- }
-
private List<ServerName> getFallBackCandidates(List<ServerName> servers) {
List<ServerName> serverNames = null;
try {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/LoadBalancerPerformanceEvaluation.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/LoadBalancerPerformanceEvaluation.java
index 45b34a1..38e19e2 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/LoadBalancerPerformanceEvaluation.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/LoadBalancerPerformanceEvaluation.java
@@ -83,7 +83,7 @@
private List<ServerName> servers;
private List<RegionInfo> regions;
private Map<RegionInfo, ServerName> regionServerMap;
- private Map<ServerName, List<RegionInfo>> serverRegionMap;
+ private Map<TableName, Map<ServerName, List<RegionInfo>>> tableServerRegionMap;
// Non-default configurations.
private void setupConf() {
@@ -92,6 +92,7 @@
}
private void generateRegionsAndServers() {
+ TableName tableName = TableName.valueOf("LoadBalancerPerfTable");
// regions
regions = new ArrayList<>(numRegions);
regionServerMap = new HashMap<>(numRegions);
@@ -101,7 +102,6 @@
Bytes.putInt(start, 0, i);
Bytes.putInt(end, 0, i + 1);
- TableName tableName = TableName.valueOf("LoadBalancerPerfTable");
RegionInfo hri = RegionInfoBuilder.newBuilder(tableName)
.setStartKey(start)
.setEndKey(end)
@@ -114,12 +114,13 @@
// servers
servers = new ArrayList<>(numServers);
- serverRegionMap = new HashMap<>(numServers);
+ Map<ServerName, List<RegionInfo>> serverRegionMap = new HashMap<>(numServers);
for (int i = 0; i < numServers; ++i) {
ServerName sn = ServerName.valueOf("srv" + i, HConstants.DEFAULT_REGIONSERVER_PORT, i);
servers.add(sn);
serverRegionMap.put(sn, i == 0 ? regions : Collections.emptyList());
}
+ tableServerRegionMap = Collections.singletonMap(tableName, serverRegionMap);
}
@Override
@@ -174,7 +175,7 @@
LOG.info("Calling " + methodName);
watch.reset().start();
- loadBalancer.balanceTable(HConstants.ENSEMBLE_TABLE_NAME, serverRegionMap);
+ loadBalancer.balanceCluster(tableServerRegionMap);
System.out.print(formatResults(methodName, watch.elapsed(TimeUnit.MILLISECONDS)));
return EXIT_SUCCESS;
