diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java index a567e1d..4f57f47 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java @@ -21,7 +21,6 @@ package org.apache.hadoop.hbase.master; import static org.apache.hadoop.hbase.util.CollectionUtils.computeIfAbsent; import com.google.common.annotations.VisibleForTesting; - import java.io.IOException; import java.net.InetAddress; import java.util.ArrayList; @@ -37,7 +36,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentNavigableMap; import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.CopyOnWriteArrayList; - +import java.util.function.Predicate; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; @@ -1075,6 +1074,27 @@ public class ServerManager { } /** + * @param keys The target server name + * @param idleServerPredicator Evaluates the server on the given load + * @return A copy of the internal list of online servers matched by the predicator + */ + public List getOnlineServersListWithIdlePredicator(List keys, + Predicate idleServerPredicator) { + List names = new ArrayList<>(); + if (keys != null) { + keys.forEach(name -> { + ServerLoad load = onlineServers.get(name); + if (load != null && idleServerPredicator != null) { + if (idleServerPredicator.test(load)) { + names.add(name); + } + } + }); + } + return names; + } + + /** * @return A copy of the internal list of draining servers. */ public List getDrainingServersList() { diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java index 807632c..f306b6f 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java @@ -17,6 +17,11 @@ */ package org.apache.hadoop.hbase.master.balancer; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Joiner; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -33,7 +38,7 @@ import java.util.NavigableMap; import java.util.Random; import java.util.Set; import java.util.TreeMap; - +import java.util.function.Predicate; import org.apache.commons.lang.NotImplementedException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -43,6 +48,7 @@ import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HDFSBlocksDistribution; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.RegionLoad; +import org.apache.hadoop.hbase.ServerLoad; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionReplicaUtil; @@ -54,12 +60,6 @@ import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster.Action.T import org.apache.hadoop.hbase.security.access.AccessControlLists; import org.apache.hadoop.util.StringUtils; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Joiner; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.Lists; -import com.google.common.collect.Sets; - /** * The base class for load balancers. It provides the the functions used to by * {@link org.apache.hadoop.hbase.master.AssignmentManager} to assign regions @@ -73,6 +73,9 @@ public abstract class BaseLoadBalancer implements LoadBalancer { private static final List EMPTY_REGION_LIST = new ArrayList(0); + static final Predicate SERVER_PREDICATOR + = load -> load.getNumberOfRegions() == 0; + protected final RegionLocationFinder regionFinder = new RegionLocationFinder(); private static class DefaultRackManager extends RackManager { @@ -1323,6 +1326,11 @@ public abstract class BaseLoadBalancer implements LoadBalancer { rackManager); } + private List findIdleServers(List servers) { + return this.services.getServerManager() + .getOnlineServersListWithIdlePredicator(servers, SERVER_PREDICATOR); + } + /** * Used to assign a single region to a random server. */ @@ -1346,10 +1354,15 @@ public abstract class BaseLoadBalancer implements LoadBalancer { if (numServers == 1) { // Only one server, nothing fancy we can do here return servers.get(0); } - + List idleServers = findIdleServers(servers); + if (idleServers.size() == 1) { + return idleServers.get(0); + } + final List finalServers = idleServers.isEmpty() ? + servers : idleServers; List regions = Lists.newArrayList(regionInfo); - Cluster cluster = createCluster(servers, regions, false); - return randomAssignment(cluster, regionInfo, servers); + Cluster cluster = createCluster(finalServers, regions, false); + return randomAssignment(cluster, regionInfo, finalServers); } /** diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java index b0b0a2b..09417cc 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java @@ -17,12 +17,9 @@ */ package org.apache.hadoop.hbase.master.balancer; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - +import com.google.common.collect.Lists; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -30,7 +27,6 @@ import java.util.Map; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; - import org.apache.commons.lang.ArrayUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -45,17 +41,21 @@ import org.apache.hadoop.hbase.master.LoadBalancer; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.RackManager; import org.apache.hadoop.hbase.master.RegionPlan; +import org.apache.hadoop.hbase.master.ServerManager; import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster; import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster.MoveRegionAction; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.net.DNSToSwitchMapping; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; import org.mockito.Mockito; - -import com.google.common.collect.Lists; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @Category({MasterTests.class, MediumTests.class}) public class TestBaseLoadBalancer extends BalancerTestBase { @@ -211,6 +211,49 @@ public class TestBaseLoadBalancer extends BalancerTestBase { assertRetainedAssignment(existing, listOfServerNames, assignment); } + @Test (timeout=30000) + public void testRandomAssignment() throws Exception { + for (int i = 1; i != 5; ++i) { + LOG.info("run testRandomAssignment() with idle servers:" + i); + testRandomAssignment(i); + } + } + + private void testRandomAssignment(int numberOfIdleServers) throws Exception { + assert numberOfIdleServers > 0; + List idleServers = new ArrayList<>(numberOfIdleServers); + for (int i = 0; i != numberOfIdleServers; ++i) { + idleServers.add(ServerName.valueOf("server-" + i, 1000, 1L)); + } + List allServers = new ArrayList<>(idleServers.size() + 1); + allServers.add(ServerName.valueOf("server-" + numberOfIdleServers, 1000, 1L)); + allServers.addAll(idleServers); + LoadBalancer balancer = new MockBalancer() { + @Override + public boolean shouldBeOnMaster(HRegionInfo region) { + return false; + } + }; + Configuration conf = HBaseConfiguration.create(); + conf.setClass("hbase.util.ip.to.rack.determiner", MockMapping.class, DNSToSwitchMapping.class); + balancer.setConf(conf); + ServerManager sm = Mockito.mock(ServerManager.class); + Mockito.when(sm.getOnlineServersListWithIdlePredicator(allServers, BaseLoadBalancer.SERVER_PREDICATOR)) + .thenReturn(idleServers); + MasterServices services = Mockito.mock(MasterServices.class); + Mockito.when(services.getServerManager()).thenReturn(sm); + balancer.setMasterServices(services); + HRegionInfo hri1 = new HRegionInfo( + TableName.valueOf("table"), "key1".getBytes(), "key2".getBytes(), + false, 100); + assertNull(balancer.randomAssignment(hri1, Collections.EMPTY_LIST)); + assertNull(balancer.randomAssignment(hri1, null)); + for (int i = 0; i != 3; ++i) { + ServerName sn = balancer.randomAssignment(hri1, allServers); + assertTrue("actual:" + sn + ", except:" + idleServers, idleServers.contains(sn)); + } + } + @Test (timeout=180000) public void testRegionAvailability() throws Exception { // Create a cluster with a few servers, assign them to specific racks