From afa9836b87f9c909ac4fc18d904a5b5b0a3dfff6 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Wed, 5 May 2021 15:48:01 +0800 Subject: [PATCH] HBASE-25834 Remove balanceTable method from LoadBalancer interface (#3217) Signed-off-by: Yulin Niu --- .../rsgroup/RSGroupBasedLoadBalancer.java | 37 +---------------- .../favored/FavoredNodeLoadBalancer.java | 2 +- .../hadoop/hbase/master/LoadBalancer.java | 17 ++------ .../master/balancer/BaseLoadBalancer.java | 41 ++++++++++++++++--- .../balancer/FavoredStochasticBalancer.java | 2 +- .../balancer/MaintenanceLoadBalancer.java | 6 --- .../master/balancer/SimpleLoadBalancer.java | 22 ++++------ .../balancer/StochasticLoadBalancer.java | 2 +- .../LoadBalancerPerformanceEvaluation.java | 9 ++-- .../master/balancer/TestBaseLoadBalancer.java | 7 +--- 10 files changed, 58 insertions(+), 87 deletions(-) diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java index 0fd522477df..a8e77c220a0 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java @@ -120,8 +120,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { } /** - * Override to balance by RSGroup - * not invoke {@link #balanceTable(TableName, Map)} + * Balance by RSGroup. */ @Override public List balanceCluster( @@ -427,40 +426,6 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer { internalBalancer.updateBalancerStatus(status); } - /** - * can achieve table balanced rather than overall balanced - */ - @Override - public List balanceTable(TableName tableName, - Map> loadOfOneTable) { - if (!isOnline()) { - LOG.error(RSGroupInfoManager.class.getSimpleName() - + " is not online, unable to perform balanceTable"); - return null; - } - Map>> loadOfThisTable = new HashMap<>(); - loadOfThisTable.put(tableName, loadOfOneTable); - Pair>>, List> - 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>> correctedLoadOfThisTable = - correctedStateAndRegionPlans.getFirst(); - List regionPlans = correctedStateAndRegionPlans.getSecond(); - List tablePlans = - this.internalBalancer.balanceTable(tableName, correctedLoadOfThisTable.get(tableName)); - - if (tablePlans != null) { - regionPlans.addAll(tablePlans); - } - return regionPlans; - } - private List getFallBackCandidates(List servers) { List serverNames = null; try { 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 888ed7e1886..67d868d1a96 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 @@ -89,7 +89,7 @@ public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements Favored } @Override - public List balanceTable(TableName tableName, + protected List balanceTable(TableName tableName, Map> loadOfOneTable) { // TODO. Look at is whether Stochastic loadbalancer can be integrated with this List plans = new ArrayList<>(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java index 752f67b4dfc..9ba68a54285 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java @@ -86,25 +86,14 @@ public interface LoadBalancer extends Configurable, Stoppable, ConfigurationObse void setMasterServices(MasterServices masterServices); /** - * Perform the major balance operation for cluster, will invoke {@link #balanceTable} to do - * actual balance. Normally not need override this method, except SimpleLoadBalancer and - * 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 balanceCluster(Map>> loadOfAllTable) throws IOException; + List balanceCluster(Map>> 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 balanceTable(TableName tableName, - Map> loadOfOneTable); /** * Perform a Round Robin assignment of regions. * @param regions diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java index f5c462f1150..3b7ec0acd6b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java @@ -773,13 +773,44 @@ public abstract class BaseLoadBalancer implements LoadBalancer { return returnMap; } - @Override - public abstract List balanceTable(TableName tableName, - Map> loadOfOneTable); + /** + * Perform the major balance operation for table, all sub classes should override this method. + *

+ * 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 balanceTable(TableName tableName, + Map> loadOfOneTable); + /** + * Called before actually executing balanceCluster. The sub classes could override this method to + * do some initialization work. + */ + protected void + preBalanceCluster(Map>> loadOfAllTable) { + } + + /** + * Perform the major balance operation for cluster, will invoke + * {@link #balanceTable(TableName, Map)} to do actual balance. + *

+ * 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 - balanceCluster(Map>> loadOfAllTable) { + public final synchronized List + balanceCluster(Map>> loadOfAllTable) { + preBalanceCluster(loadOfAllTable); if (isByTable) { List result = new ArrayList<>(); loadOfAllTable.forEach((tableName, loadOfOneTable) -> { 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 df047dc6f2f..81c0a2ce260 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 @@ -700,7 +700,7 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements * implementation. For the misplaced regions, we assign a bogus server to it and AM takes care. */ @Override - public synchronized List balanceTable(TableName tableName, + protected List balanceTable(TableName tableName, Map> loadOfOneTable) { if (this.services != null) { 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 5c25272d153..4a3cd4954a7 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 @@ -67,12 +67,6 @@ public class MaintenanceLoadBalancer extends Configured implements LoadBalancer return Collections.emptyList(); } - @Override - public List balanceTable(TableName tableName, - Map> loadOfOneTable) { - return Collections.emptyList(); - } - private Map> assign(Collection regions, List servers) { // should only have 1 region server in maintenance mode diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java index f25585493d7..c5da3bf538b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/SimpleLoadBalancer.java @@ -126,6 +126,13 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { avgLoadOverall = sum / serverLoadList.size(); } + @Override + protected void + preBalanceCluster(Map>> 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; @@ -251,7 +258,7 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { * or null if cluster is already balanced */ @Override - public List balanceTable(TableName tableName, + protected List balanceTable(TableName tableName, Map> loadOfOneTable) { List regionsToReturn = balanceMasterRegions(loadOfOneTable); if (regionsToReturn != null || loadOfOneTable == null || loadOfOneTable.size() <= 1) { @@ -485,7 +492,7 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { * 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 regionsToReturn, + private void balanceOverall(List regionsToReturn, Map serverBalanceInfo, boolean fetchFromTail, MinMaxPriorityQueue regionsToMove, int max, int min) { // Step 1. @@ -632,15 +639,4 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { 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 - balanceCluster(Map>> loadOfAllTable) { - setClusterLoad(loadOfAllTable); - return super.balanceCluster(loadOfAllTable); - } } 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 c43ca752b33..685f247f86d 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 @@ -377,7 +377,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { * should always approach the optimal state given enough steps. */ @Override - public synchronized List balanceTable(TableName tableName, Map balanceTable(TableName tableName, Map> loadOfOneTable) { List plans = balanceMasterRegions(loadOfOneTable); if (plans != null || loadOfOneTable == null || loadOfOneTable.size() <= 1) { 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 45b34a1d491..38e19e2a1bc 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 @@ public class LoadBalancerPerformanceEvaluation extends AbstractHBaseTool { private List servers; private List regions; private Map regionServerMap; - private Map> serverRegionMap; + private Map>> tableServerRegionMap; // Non-default configurations. private void setupConf() { @@ -92,6 +92,7 @@ public class LoadBalancerPerformanceEvaluation extends AbstractHBaseTool { } private void generateRegionsAndServers() { + TableName tableName = TableName.valueOf("LoadBalancerPerfTable"); // regions regions = new ArrayList<>(numRegions); regionServerMap = new HashMap<>(numRegions); @@ -101,7 +102,6 @@ public class LoadBalancerPerformanceEvaluation extends AbstractHBaseTool { 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 @@ public class LoadBalancerPerformanceEvaluation extends AbstractHBaseTool { // servers servers = new ArrayList<>(numServers); - serverRegionMap = new HashMap<>(numServers); + Map> 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 @@ public class LoadBalancerPerformanceEvaluation extends AbstractHBaseTool { 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; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java index 54078a5b70a..63b6791682b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java @@ -114,14 +114,9 @@ public class TestBaseLoadBalancer extends BalancerTestBase { } public static class MockBalancer extends BaseLoadBalancer { - @Override - public List - balanceCluster(Map>> loadOfAllTable) { - return null; - } @Override - public List balanceTable(TableName tableName, + protected List balanceTable(TableName tableName, Map> loadOfOneTable) { return null; }