diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 6e2b921..be2c777 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -1479,7 +1479,7 @@ MasterServices, Server { long cutoffTime = System.currentTimeMillis() + maximumBalanceTime; int rpCount = 0; // number of RegionPlans balanced so far long totalRegPlanExecTime = 0; - balancerRan = plans != null; + balancerRan = plans.size() != 0; if (plans != null && !plans.isEmpty()) { for (RegionPlan plan: plans) { LOG.info("balance " + plan); 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 8fdd37c..d7ba4ec 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 @@ -131,6 +131,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { int numMovedRegions = 0; //num moved regions from the initial configuration int numMovedMetaRegions = 0; //num of moved regions that are META + Map> clusterState; protected final RackManager rackManager; @@ -163,6 +164,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { List> serversPerHostList = new ArrayList>(); List> serversPerRackList = new ArrayList>(); + this.clusterState = clusterState; // Use servername and port as there can be dead servers in this list. We want everything with // a matching hostname and port to have the same index. @@ -821,7 +823,8 @@ public abstract class BaseLoadBalancer implements LoadBalancer { this.rackManager = rackManager; } - protected boolean needsBalance(ClusterLoadState cs) { + protected boolean needsBalance(Cluster c) { + ClusterLoadState cs = new ClusterLoadState(c.clusterState); if (cs.getNumServers() < MIN_SERVER_BALANCE) { if (LOG.isDebugEnabled()) { LOG.debug("Not running balancer because only " + cs.getNumServers() @@ -829,8 +832,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { } return false; } - // TODO: check for co-located region replicas as well - + if(areSomeRegionReplicasColocated(c)) return true; // Check if we even need to do any load balancing // HBASE-3681 check sloppiness first float average = cs.getLoadAverage(); // for logging @@ -852,6 +854,17 @@ public abstract class BaseLoadBalancer implements LoadBalancer { } /** + * Subclasses should implement this to return true if the cluster has nodes that hosts + * multiple replicas for the same region, or, if there are multiple racks and the same + * rack hosts replicas of the same region + * @param c + * @return + */ + protected boolean areSomeRegionReplicasColocated(Cluster c) { + return false; + } + + /** * Generates a bulk assignment plan to be used on cluster startup using a * simple round-robin assignment. *

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 da6b443..930dc96 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 @@ -184,8 +184,10 @@ public class SimpleLoadBalancer extends BaseLoadBalancer { long startTime = System.currentTimeMillis(); ClusterLoadState cs = new ClusterLoadState(clusterMap); - - if (!this.needsBalance(cs)) return null; + // construct a Cluster object with clusterMap and rest of the + // argument as defaults + Cluster c = new Cluster(clusterMap, null, this.regionFinder, this.rackManager); + if (!this.needsBalance(c)) return null; int numServers = cs.getNumServers(); NavigableMap> serversByLoad = cs.getServersByLoad(); 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 3da6266..0ffbc2e 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 @@ -123,6 +123,8 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { // when new services are offered private LocalityBasedCandidateGenerator localityCandidateGenerator; private LocalityCostFunction localityCost; + private RegionReplicaHostCostFunction regionReplicaHostCostFunction; + private RegionReplicaRackCostFunction regionReplicaRackCostFunction; @Override public void setConf(Configuration conf) { @@ -152,13 +154,16 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { new StoreFileCostFunction(conf) }; + regionReplicaHostCostFunction = new RegionReplicaHostCostFunction(conf); + regionReplicaRackCostFunction = new RegionReplicaRackCostFunction(conf); + costFunctions = new CostFunction[]{ new RegionCountSkewCostFunction(conf), new MoveCostFunction(conf), localityCost, new TableSkewCostFunction(conf), - new RegionReplicaHostCostFunction(conf), - new RegionReplicaRackCostFunction(conf), + regionReplicaHostCostFunction, + regionReplicaRackCostFunction, regionLoadFunctions[0], regionLoadFunctions[1], regionLoadFunctions[2], @@ -189,6 +194,15 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { } + @Override + protected boolean areSomeRegionReplicasColocated(Cluster c) { + regionReplicaHostCostFunction.init(c); + if (regionReplicaHostCostFunction.cost() > 0) return true; + regionReplicaRackCostFunction.init(c); + if (regionReplicaRackCostFunction.cost() > 0) return true; + return false; + } + /** * Given the cluster state this will try and approach an optimal balance. This * should always approach the optimal state given enough steps. @@ -197,14 +211,14 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { public List balanceCluster(Map> clusterState) { //The clusterState that is given to this method contains the state //of all the regions in the table(s) (that's true today) - if (!needsBalance(new ClusterLoadState(clusterState))) { + // Keep track of servers to iterate through them. + Cluster cluster = new Cluster(clusterState, loads, regionFinder, rackManager); + if (!needsBalance(cluster)) { return null; } long startTime = EnvironmentEdgeManager.currentTimeMillis(); - // Keep track of servers to iterate through them. - Cluster cluster = new Cluster(clusterState, loads, regionFinder, rackManager); initCosts(cluster); double currentCost = computeCost(cluster, Double.MAX_VALUE); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java index 0294b12..0e9a321 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java @@ -118,10 +118,14 @@ public class TestRegionRebalancing { UTIL.getHBaseCluster().getMaster().balance(); assertRegionsAreBalanced(); + // On a balanced cluster, calling balance() should return false + assert(UTIL.getHBaseCluster().getMaster().balance() == false); + + // However if we add a server, then the balance() call should return true // add a region server - total of 3 LOG.info("Started third server=" + UTIL.getHBaseCluster().startRegionServer().getRegionServer().getServerName()); - UTIL.getHBaseCluster().getMaster().balance(); + assert(UTIL.getHBaseCluster().getMaster().balance() == true); assertRegionsAreBalanced(); // kill a region server - total of 2 @@ -135,14 +139,14 @@ public class TestRegionRebalancing { UTIL.getHBaseCluster().startRegionServer().getRegionServer().getServerName()); LOG.info("Added fourth server=" + UTIL.getHBaseCluster().startRegionServer().getRegionServer().getServerName()); - UTIL.getHBaseCluster().getMaster().balance(); + assert(UTIL.getHBaseCluster().getMaster().balance() == true); assertRegionsAreBalanced(); for (int i = 0; i < 6; i++){ LOG.info("Adding " + (i + 5) + "th region server"); UTIL.getHBaseCluster().startRegionServer(); } - UTIL.getHBaseCluster().getMaster().balance(); + assert(UTIL.getHBaseCluster().getMaster().balance() == true); assertRegionsAreBalanced(); table.close(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java index 7ffb7d5..e9e9a36 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -46,6 +47,7 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.client.RegionReplicaUtil; import org.apache.hadoop.hbase.master.RackManager; import org.apache.hadoop.hbase.master.RegionPlan; +import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.net.DNSToSwitchMapping; import org.apache.hadoop.net.NetworkTopology; @@ -382,6 +384,34 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { assertTrue(costWith2ReplicasOnTwoServers < costWith3ReplicasSameServer); } + @Test + public void testNeedsBalanceForColocatedReplicas() { + // check for the case where there are two hosts and with one rack, and where + // both the replicas are hosted on the same server + List regions = randomRegions(1); + ServerName s1 = ServerName.valueOf("host1", 1000, 11111); + ServerName s2 = ServerName.valueOf("host11", 1000, 11111); + Map> map = new HashMap>(); + map.put(s1, regions); + regions.add(RegionReplicaUtil.getRegionInfoForReplica(regions.get(0), 1)); + // until the step above s1 holds two replicas of a region + regions = randomRegions(1); + map.put(s2, regions); + assertTrue(loadBalancer.needsBalance(new Cluster(map, null, null, null))); + // check for the case where there are two hosts on the same rack and there are two racks + // and both the replicas are on the same rack + map.clear(); + regions = randomRegions(1); + List regionsOnS2 = new ArrayList(1); + regionsOnS2.add(RegionReplicaUtil.getRegionInfoForReplica(regions.get(0), 1)); + map.put(s1, regions); + map.put(s2, regionsOnS2); + // add another server so that the cluster has some host on another rack + map.put(ServerName.valueOf("host2", 1000, 11111), randomRegions(1)); + assertTrue(loadBalancer.needsBalance(new Cluster(map, null, null, + new ForTestRackManagerOne()))); + } + @Test (timeout = 60000) public void testSmallCluster() { int numNodes = 10; @@ -546,6 +576,13 @@ public class TestStochasticLoadBalancer extends BalancerTestBase { } } + private static class ForTestRackManagerOne extends RackManager { + @Override + public String getRack(ServerName server) { + return server.getHostname().endsWith("1") ? "rack1" : "rack2"; + } + } + @Test (timeout = 120000) public void testRegionReplicationOnMidClusterWithRacks() { conf.setLong(StochasticLoadBalancer.MAX_STEPS_KEY, 4000000L);