diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesManager.java index dbba5c9b916..7705b3d1b74 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesManager.java @@ -46,6 +46,7 @@ import org.apache.hadoop.hdfs.HdfsConfiguration; import org.apache.hadoop.net.NetUtils; import org.apache.yetus.audience.InterfaceAudience; +import org.apache.hadoop.hbase.shaded.com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hbase.shaded.com.google.common.collect.Lists; import org.apache.hadoop.hbase.shaded.com.google.common.collect.Maps; import org.apache.hadoop.hbase.shaded.com.google.common.collect.Sets; @@ -287,6 +288,23 @@ public class FavoredNodesManager { } } + @VisibleForTesting + public synchronized Set getRegionsOfFavoredNode(ServerName serverName) { + Set regionInfos = Sets.newHashSet(); + + ServerName serverToUse = ServerName.valueOf(serverName.getHostAndPort(), NON_STARTCODE); + if (primaryRSToRegionMap.containsKey(serverToUse)) { + regionInfos.addAll(primaryRSToRegionMap.get(serverToUse)); + } + if (secondaryRSToRegionMap.containsKey(serverToUse)) { + regionInfos.addAll(secondaryRSToRegionMap.get(serverToUse)); + } + if (teritiaryRSToRegionMap.containsKey(serverToUse)) { + regionInfos.addAll(teritiaryRSToRegionMap.get(serverToUse)); + } + return regionInfos; + } + public RackManager getRackManager() { return rackManager; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/favored/TestFavoredNodeAssignmentHelper.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/favored/TestFavoredNodeAssignmentHelper.java index 24bb4bd00af..ffb39f8640a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/favored/TestFavoredNodeAssignmentHelper.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/favored/TestFavoredNodeAssignmentHelper.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java index 69baa5ff493..b300818047a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java @@ -54,13 +54,11 @@ public class TestMasterMetrics { public MyMaster(Configuration conf) throws IOException, KeeperException, InterruptedException { super(conf); } -/* @Override protected void tryRegionServerReport( long reportStartTime, long reportEndTime) { // do nothing } -*/ } @BeforeClass diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestFavoredStochasticBalancerPickers.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestFavoredStochasticBalancerPickers.java index e636cb02a4c..5a0951997d2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestFavoredStochasticBalancerPickers.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestFavoredStochasticBalancerPickers.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -23,7 +23,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; import java.util.List; @@ -35,26 +34,36 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.ClusterStatus; import org.apache.hadoop.hbase.ClusterStatus.Option; import org.apache.hadoop.hbase.HBaseTestingUtility; -import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.MiniHBaseCluster; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.favored.FavoredNodesManager; +import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.LoadBalancer; import org.apache.hadoop.hbase.master.RackManager; +import org.apache.hadoop.hbase.master.assignment.RegionStates; import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster; +import org.apache.hadoop.hbase.regionserver.HRegion; +import org.apache.hadoop.hbase.regionserver.HRegionServer; +import org.apache.hadoop.hbase.regionserver.Region; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.JVMClusterUtil; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; -import org.junit.experimental.categories.Category; import org.apache.hadoop.hbase.shaded.com.google.common.collect.Lists; import org.apache.hadoop.hbase.shaded.com.google.common.collect.Maps; @@ -70,6 +79,10 @@ public class TestFavoredStochasticBalancerPickers extends BalancerTestBase { private static Configuration conf; private Admin admin; + private MiniHBaseCluster cluster; + + @Rule + public TestName name = new TestName(); @BeforeClass public static void setupBeforeClass() throws Exception { @@ -88,6 +101,7 @@ public class TestFavoredStochasticBalancerPickers extends BalancerTestBase { TEST_UTIL.startMiniCluster(SLAVES); TEST_UTIL.getDFSCluster().waitClusterUp(); TEST_UTIL.getHBaseCluster().waitForActiveAndReadyMaster(120*1000); + cluster = TEST_UTIL.getMiniHBaseCluster(); admin = TEST_UTIL.getAdmin(); admin.setBalancerRunning(false, true); } @@ -99,34 +113,56 @@ public class TestFavoredStochasticBalancerPickers extends BalancerTestBase { } - @Ignore @Test + @Test public void testPickers() throws Exception { - - TableName tableName = TableName.valueOf("testPickers"); - HTableDescriptor desc = new HTableDescriptor(tableName); - desc.addFamily(new HColumnDescriptor(HConstants.CATALOG_FAMILY)); + TableName tableName = TableName.valueOf(name.getMethodName()); + ColumnFamilyDescriptor columnFamilyDescriptor = + ColumnFamilyDescriptorBuilder.newBuilder(HConstants.CATALOG_FAMILY).build(); + TableDescriptor desc = TableDescriptorBuilder + .newBuilder(tableName) + .addColumnFamily(columnFamilyDescriptor) + .build(); admin.createTable(desc, Bytes.toBytes("aaa"), Bytes.toBytes("zzz"), REGIONS); + TEST_UTIL.waitUntilAllRegionsAssigned(tableName); TEST_UTIL.loadTable(admin.getConnection().getTable(tableName), HConstants.CATALOG_FAMILY); admin.flush(tableName); - ServerName masterServerName = TEST_UTIL.getMiniHBaseCluster().getServerHoldingMeta(); - final ServerName mostLoadedServer = getRSWithMaxRegions(Lists.newArrayList(masterServerName)); + HMaster master = cluster.getMaster(); + FavoredNodesManager fnm = master.getFavoredNodesManager(); + ServerName masterServerName = master.getServerName(); + List excludedServers = Lists.newArrayList(masterServerName); + final ServerName mostLoadedServer = getRSWithMaxRegions(tableName, excludedServers); assertNotNull(mostLoadedServer); - int numRegions = admin.getOnlineRegions(mostLoadedServer).size(); - ServerName source = getRSWithMaxRegions(Lists.newArrayList(masterServerName, mostLoadedServer)); + int numRegions = getTableRegionsFromServer(tableName, mostLoadedServer).size(); + excludedServers.add(mostLoadedServer); + // Lets find another server with more regions to calculate number of regions to move + ServerName source = getRSWithMaxRegions(tableName, excludedServers); assertNotNull(source); - int regionsToMove = admin.getOnlineRegions(source).size()/2; - List hris = admin.getRegions(source); + int regionsToMove = getTableRegionsFromServer(tableName, source).size()/2; + + // Since move only works if the target is part of favored nodes of the region, lets get all + // regions that are movable to mostLoadedServer + List hris = getRegionsThatCanBeMoved(tableName, mostLoadedServer); + RegionStates rst = master.getAssignmentManager().getRegionStates(); for (int i = 0; i < regionsToMove; i++) { - admin.move(hris.get(i).getEncodedNameAsBytes(), Bytes.toBytes(mostLoadedServer.getServerName())); + final RegionInfo regionInfo = hris.get(i); + admin.move(regionInfo.getEncodedNameAsBytes(), + Bytes.toBytes(mostLoadedServer.getServerName())); LOG.info("Moving region: " + hris.get(i).getRegionNameAsString() + " to " + mostLoadedServer); + TEST_UTIL.waitFor(60000, new Waiter.Predicate() { + @Override + public boolean evaluate() throws Exception { + return ServerName.isSameAddress( + rst.getRegionServerOfRegion(regionInfo), mostLoadedServer); + } + }); } final int finalRegions = numRegions + regionsToMove; TEST_UTIL.waitUntilNoRegionsInTransition(60000); TEST_UTIL.waitFor(60000, new Waiter.Predicate() { @Override public boolean evaluate() throws Exception { - int numRegions = TEST_UTIL.getAdmin().getOnlineRegions(mostLoadedServer).size(); + int numRegions = getTableRegionsFromServer(tableName, mostLoadedServer).size(); return (numRegions == finalRegions); } }); @@ -136,7 +172,7 @@ public class TestFavoredStochasticBalancerPickers extends BalancerTestBase { ClusterStatus status = admin.getClusterStatus(EnumSet.of(Option.LIVE_SERVERS)); for (ServerName sn : status.getServers()) { if (!ServerName.isSameAddress(sn, masterServerName)) { - serverAssignments.put(sn, admin.getRegions(sn)); + serverAssignments.put(sn, getTableRegionsFromServer(tableName, sn)); } } RegionLocationFinder regionFinder = new RegionLocationFinder(); @@ -146,7 +182,7 @@ public class TestFavoredStochasticBalancerPickers extends BalancerTestBase { Cluster cluster = new Cluster(serverAssignments, null, regionFinder, new RackManager(conf)); LoadOnlyFavoredStochasticBalancer balancer = (LoadOnlyFavoredStochasticBalancer) TEST_UTIL .getMiniHBaseCluster().getMaster().getLoadBalancer(); - FavoredNodesManager fnm = TEST_UTIL.getMiniHBaseCluster().getMaster().getFavoredNodesManager(); + cluster.sortServersByRegionCount(); Integer[] servers = cluster.serverIndicesSortedByRegionCount; LOG.info("Servers sorted by region count:" + Arrays.toString(servers)); @@ -180,22 +216,55 @@ public class TestFavoredStochasticBalancerPickers extends BalancerTestBase { assertTrue("load picker did not pick expected regions in 100 iterations.", userRegionPicked); } - private ServerName getRSWithMaxRegions(ArrayList excludeNodes) throws IOException { + /* + * A region can only be moved to one of its favored node. Hence this method helps us to + * get that list which makes it easy to write non-flaky tests. + */ + private List getRegionsThatCanBeMoved(TableName tableName, + ServerName serverName) { + List regions = Lists.newArrayList(); + RegionStates rst = cluster.getMaster().getAssignmentManager().getRegionStates(); + FavoredNodesManager fnm = cluster.getMaster().getFavoredNodesManager(); + for (RegionInfo regionInfo : fnm.getRegionsOfFavoredNode(serverName)) { + if (regionInfo.getTable().equals(tableName) && + !ServerName.isSameAddress(rst.getRegionServerOfRegion(regionInfo), serverName)) { + regions.add(regionInfo); + } + } + return regions; + } + + private List getTableRegionsFromServer(TableName tableName, ServerName source) + throws IOException { + List regionInfos = Lists.newArrayList(); + HRegionServer regionServer = cluster.getRegionServer(source); + for (Region region : regionServer.getRegions(tableName)) { + regionInfos.add(region.getRegionInfo()); + } + return regionInfos; + } + + private ServerName getRSWithMaxRegions(TableName tableName, List excludeNodes) + throws IOException { + int maxRegions = 0; ServerName maxLoadedServer = null; - for (ServerName sn : admin.getClusterStatus(EnumSet.of(Option.LIVE_SERVERS)).getServers()) { - if (admin.getOnlineRegions(sn).size() > maxRegions) { - if (excludeNodes == null || !doesMatchExcludeNodes(excludeNodes, sn)) { - maxRegions = admin.getOnlineRegions(sn).size(); - maxLoadedServer = sn; + for (JVMClusterUtil.RegionServerThread rst : cluster.getLiveRegionServerThreads()) { + List regions = rst.getRegionServer().getRegions(tableName); + LOG.debug("Server: " + rst.getRegionServer().getServerName() + " regions: " + regions.size()); + if (regions.size() > maxRegions) { + if (excludeNodes == null || + !doesMatchExcludeNodes(excludeNodes, rst.getRegionServer().getServerName())) { + maxRegions = regions.size(); + maxLoadedServer = rst.getRegionServer().getServerName(); } } } return maxLoadedServer; } - private boolean doesMatchExcludeNodes(ArrayList excludeNodes, ServerName sn) { + private boolean doesMatchExcludeNodes(List excludeNodes, ServerName sn) { for (ServerName excludeSN : excludeNodes) { if (ServerName.isSameAddress(sn, excludeSN)) { return true;