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 491b5fabe42a..a6f5357d3797 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 @@ public interface LoadBalancer extends Configurable, Stoppable, ConfigurationObse 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 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); + List balanceCluster(Map>> 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 56fb96e6f9ee..93a733126a1b 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 @@ private Map> toEnsumbleTableLoad( 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-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 a8b161a98a1a..57fae1193631 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 @@ -122,6 +122,13 @@ void setClusterLoad(Map>> clusterLoa 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; @@ -247,7 +254,7 @@ private boolean overallNeedsBalance() { * or null if cluster is already balanced */ @Override - public List balanceTable(TableName tableName, + protected List balanceTable(TableName tableName, Map> loadOfOneTable) { long startTime = System.currentTimeMillis(); @@ -466,7 +473,7 @@ public List balanceTable(TableName tableName, * 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. @@ -613,15 +620,4 @@ private void addRegionPlan(final MinMaxPriorityQueue regionsToMove, 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-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 2e33cf19a534..a6ae2acf88a6 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 void beforeAllTests() throws Exception { } 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; } 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 aa3642affe60..17b2863bdb4b 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 @@ public synchronized void initialize() throws HBaseIOException { } @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/balancer/FavoredStochasticBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java index 4cb873e0a4ec..742863def013 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 @@ private int pickMostLoadedServer(final BalancerClusterState cluster) { * implementation. For the misplaced regions, we assign a bogus server to it and AM takes care. */ @Override - public List balanceTable(TableName tableName, + protected List balanceTable(TableName tableName, Map> loadOfOneTable) { if (this.services != null) { List 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 d009e3b326b6..11a210297352 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 @@ public List balanceCluster( 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/StochasticLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java index 0ffdc43654ac..9a7823daa504 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 @@ BalanceAction nextAction(BalancerClusterState cluster) { * should always approach the optimal state given enough steps. */ @Override - public synchronized List balanceTable(TableName tableName, Map balanceTable(TableName tableName, Map> 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 bedc77685bd5..fe82e2579be6 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 @@ public void setMasterServices(MasterServices masterServices) { } /** - * Override to balance by RSGroup - * not invoke {@link #balanceTable(TableName, Map)} + * Balance by RSGroup. */ @Override public List balanceCluster( @@ -449,40 +448,6 @@ public void updateBalancerStatus(boolean status) { 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/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 45b34a1d491f..38e19e2a1bc2 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 @@ private void setupConf() { } private void generateRegionsAndServers() { + TableName tableName = TableName.valueOf("LoadBalancerPerfTable"); // regions regions = new ArrayList<>(numRegions); regionServerMap = new HashMap<>(numRegions); @@ -101,7 +102,6 @@ private void generateRegionsAndServers() { 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 @@ private void generateRegionsAndServers() { // 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 @@ protected int doWork() throws Exception { 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;