You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2019/01/20 01:05:10 UTC
[hbase] branch branch-2.0 updated: HBASE-21746 Fix two concern
cases in RegionMover
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 389360a HBASE-21746 Fix two concern cases in RegionMover
389360a is described below
commit 389360a9743513d196382cdca36bab7e36fce6d8
Author: zhangduo <zh...@apache.org>
AuthorDate: Sun Jan 20 08:56:45 2019 +0800
HBASE-21746 Fix two concern cases in RegionMover
Signed-off-by: Guanghao Zhang <zg...@apache.org>
---
.../org/apache/hadoop/hbase/util/RegionMover.java | 30 +--
.../apache/hadoop/hbase/util/TestRegionMover.java | 235 ++++++++++-----------
2 files changed, 132 insertions(+), 133 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java
index 24c8fc3..70a4f0e 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java
@@ -45,13 +45,12 @@ import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.function.Predicate;
-
import org.apache.commons.io.IOUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.ClusterMetrics.Option;
import org.apache.hadoop.hbase.HBaseConfiguration;
import org.apache.hadoop.hbase.HConstants;
-import org.apache.hadoop.hbase.MetaTableAccessor;
+import org.apache.hadoop.hbase.HRegionLocation;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.Connection;
@@ -667,18 +666,15 @@ public class RegionMover extends AbstractHBaseTool implements Closeable {
* @return server removed from list of Region Servers
*/
private ServerName stripServer(List<ServerName> regionServers, String hostname, int port) {
- ServerName server = null;
- String portString = Integer.toString(port);
- Iterator<ServerName> i = regionServers.iterator();
- while (i.hasNext()) {
- server = i.next();
- String[] splitServer = server.getServerName().split(ServerName.SERVERNAME_SEPARATOR);
- if (splitServer[0].equalsIgnoreCase(hostname) && splitServer[1].equals(portString)) {
- i.remove();
+ for (Iterator<ServerName> iter = regionServers.iterator(); iter.hasNext();) {
+ ServerName server = iter.next();
+ if (server.getAddress().getHostname().equalsIgnoreCase(hostname) &&
+ server.getAddress().getPort() == port) {
+ iter.remove();
return server;
}
}
- return server;
+ return null;
}
/**
@@ -719,7 +715,13 @@ public class RegionMover extends AbstractHBaseTool implements Closeable {
if (!admin.isTableEnabled(region.getTable())) {
return null;
}
- return MetaTableAccessor.getRegionLocation(conn, region).getServerName();
+ HRegionLocation loc =
+ conn.getRegionLocator(region.getTable()).getRegionLocation(region.getStartKey(), true);
+ if (loc != null) {
+ return loc.getServerName();
+ } else {
+ return null;
+ }
}
@Override
@@ -782,6 +784,8 @@ public class RegionMover extends AbstractHBaseTool implements Closeable {
}
public static void main(String[] args) {
- new RegionMover().doStaticMain(args);
+ try (RegionMover mover = new RegionMover()) {
+ mover.doStaticMain(args);
+ }
}
}
\ No newline at end of file
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java
index dd92b95..670248d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java
@@ -18,21 +18,25 @@
package org.apache.hadoop.hbase.util;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import java.io.File;
import java.io.FileWriter;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.HBaseClassTestRule;
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.TableName;
import org.apache.hadoop.hbase.Waiter.Predicate;
import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.regionserver.HRegionServer;
import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.MiscTests;
import org.apache.hadoop.hbase.util.RegionMover.RegionMoverBuilder;
import org.junit.AfterClass;
import org.junit.Before;
@@ -43,24 +47,25 @@ import org.junit.experimental.categories.Category;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
/**
* Tests for Region Mover Load/Unload functionality with and without ack mode and also to test
* exclude functionality useful for rack decommissioning
*/
-@Category(MediumTests.class)
+@Category({ MiscTests.class, MediumTests.class })
public class TestRegionMover {
@ClassRule
public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestRegionMover.class);
+ HBaseClassTestRule.forClass(TestRegionMover.class);
+
+ private static final Logger LOG = LoggerFactory.getLogger(TestRegionMover.class);
- final Logger LOG = LoggerFactory.getLogger(getClass());
- protected final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+ private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
@BeforeClass
public static void setUpBeforeClass() throws Exception {
TEST_UTIL.startMiniCluster(3);
+ TEST_UTIL.getAdmin().balancerSwitch(false, true);
}
@AfterClass
@@ -76,124 +81,64 @@ public class TestRegionMover {
if (admin.tableExists(tableName)) {
TEST_UTIL.deleteTable(tableName);
}
- HTableDescriptor tableDesc = new HTableDescriptor(tableName);
- HColumnDescriptor fam1 = new HColumnDescriptor("fam1");
- tableDesc.addFamily(fam1);
-
- try {
- admin.setBalancerRunning(false, true);
- String startKey = "a";
- String endKey = "z";
- admin.createTable(tableDesc, startKey.getBytes(), endKey.getBytes(), 9);
- } finally {
- if (admin != null) {
- admin.close();
- }
- }
+ TableDescriptor tableDesc = TableDescriptorBuilder.newBuilder(tableName)
+ .setColumnFamily(ColumnFamilyDescriptorBuilder.of("fam1")).build();
+ String startKey = "a";
+ String endKey = "z";
+ admin.createTable(tableDesc, startKey.getBytes(), endKey.getBytes(), 9);
}
@Test
- public void testLoadWithAck() throws Exception {
+ public void testWithAck() throws Exception {
MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
HRegionServer regionServer = cluster.getRegionServer(0);
- String rsName = regionServer.getServerName().getHostname();
- int port = regionServer.getServerName().getPort();
- int noRegions = regionServer.getNumberOfOnlineRegions();
- String rs = rsName + ":" + Integer.toString(port);
- RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration())
- .ack(true).maxthreads(8);
- RegionMover rm = rmBuilder.build();
- LOG.info("Unloading " + rs);
- rm.unload();
- assertEquals(0, regionServer.getNumberOfOnlineRegions());
- LOG.info("Successfully Unloaded\nNow Loading");
- rm.load();
- assertEquals(noRegions, regionServer.getNumberOfOnlineRegions());
- }
-
- /** Test to unload a regionserver first and then load it using no Ack mode
- * we check if some regions are loaded on the region server(since no ack is best effort)
- */
- @Test
- public void testLoadWithoutAck() throws Exception {
- MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
- final HRegionServer regionServer = cluster.getRegionServer(0);
- String rsName = regionServer.getServerName().getHostname();
- int port = regionServer.getServerName().getPort();
- int noRegions = regionServer.getNumberOfOnlineRegions();
- String rs = rsName + ":" + Integer.toString(port);
- RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration())
- .ack(true);
- RegionMover rm = rmBuilder.build();
- LOG.info("Unloading " + rs);
- rm.unload();
- assertEquals(0, regionServer.getNumberOfOnlineRegions());
- LOG.info("Successfully Unloaded\nNow Loading");
- rm = rmBuilder.ack(false).build();
- rm.load();
- TEST_UTIL.waitFor(5000, 500, new Predicate<Exception>() {
- @Override
- public boolean evaluate() throws Exception {
- return regionServer.getNumberOfOnlineRegions() > 0;
- }
- });
- }
-
- @Test
- public void testUnloadWithoutAck() throws Exception {
- MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
- final HRegionServer regionServer = cluster.getRegionServer(0);
- final int noRegions = regionServer.getNumberOfOnlineRegions();
- String rsName = regionServer.getServerName().getHostname();
- int port = regionServer.getServerName().getPort();
- String rs = rsName + ":" + Integer.toString(port);
- RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration())
- .ack(false);
- RegionMover rm = rmBuilder.build();
- LOG.info("Unloading " + rs);
- rm.unload();
- TEST_UTIL.waitFor(5000, 500, new Predicate<Exception>() {
- @Override
- public boolean evaluate() throws Exception {
- return regionServer.getNumberOfOnlineRegions() < noRegions;
- }
- });
- }
-
- @Test
- public void testUnloadWithAck() throws Exception {
- MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
- HRegionServer regionServer = cluster.getRegionServer(0);
- String rsName = regionServer.getServerName().getHostname();
- int port = regionServer.getServerName().getPort();
- String rs = rsName + ":" + Integer.toString(port);
- RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration())
- .ack(true);
- RegionMover rm = rmBuilder.build();
- rm.unload();
- LOG.info("Unloading " + rs);
- assertEquals(0, regionServer.getNumberOfOnlineRegions());
+ String rsName = regionServer.getServerName().getAddress().toString();
+ int numRegions = regionServer.getNumberOfOnlineRegions();
+ RegionMoverBuilder rmBuilder =
+ new RegionMoverBuilder(rsName, TEST_UTIL.getConfiguration()).ack(true).maxthreads(8);
+ try (RegionMover rm = rmBuilder.build()) {
+ LOG.info("Unloading " + regionServer.getServerName());
+ rm.unload();
+ assertEquals(0, regionServer.getNumberOfOnlineRegions());
+ LOG.info("Successfully Unloaded\nNow Loading");
+ rm.load();
+ assertEquals(numRegions, regionServer.getNumberOfOnlineRegions());
+ // Repeat the same load. It should be very fast because all regions are already moved.
+ rm.load();
+ }
}
/**
- * Test that loading the same region set doesn't cause timeout loop during meta load.
+ * Test to unload a regionserver first and then load it using no Ack mode.
*/
@Test
- public void testRepeatedLoad() throws Exception {
+ public void testWithoutAck() throws Exception {
MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
HRegionServer regionServer = cluster.getRegionServer(0);
- String rsName = regionServer.getServerName().getHostname();
- int port = regionServer.getServerName().getPort();
- String rs = rsName + ":" + Integer.toString(port);
- RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration())
- .ack(true);
- RegionMover rm = rmBuilder.build();
- rm.unload();
- assertEquals(0, regionServer.getNumberOfOnlineRegions());
- rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration()).ack(true);
- rm = rmBuilder.build();
- rm.load();
- rm.load(); //Repeat the same load. It should be very fast because all regions are already moved.
+ String rsName = regionServer.getServerName().getAddress().toString();
+ int numRegions = regionServer.getNumberOfOnlineRegions();
+ RegionMoverBuilder rmBuilder =
+ new RegionMoverBuilder(rsName, TEST_UTIL.getConfiguration()).ack(false);
+ try (RegionMover rm = rmBuilder.build()) {
+ LOG.info("Unloading " + regionServer.getServerName());
+ rm.unload();
+ TEST_UTIL.waitFor(30000, 1000, new Predicate<Exception>() {
+ @Override
+ public boolean evaluate() throws Exception {
+ return regionServer.getNumberOfOnlineRegions() == 0;
+ }
+ });
+ LOG.info("Successfully Unloaded\nNow Loading");
+ rm.load();
+ // In UT we only have 10 regions so it is not likely to fail, so here we check for all
+ // regions, in the real production this may not be true.
+ TEST_UTIL.waitFor(30000, 1000, new Predicate<Exception>() {
+ @Override
+ public boolean evaluate() throws Exception {
+ return regionServer.getNumberOfOnlineRegions() == numRegions;
+ }
+ });
+ }
}
/**
@@ -218,13 +163,14 @@ public class TestRegionMover {
String rs = rsName + ":" + Integer.toString(port);
RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration())
.ack(true).excludeFile(excludeFile.getCanonicalPath());
- RegionMover rm = rmBuilder.build();
- rm.unload();
- LOG.info("Unloading " + rs);
- assertEquals(0, regionServer.getNumberOfOnlineRegions());
- assertEquals(regionsExcludeServer, cluster.getRegionServer(1).getNumberOfOnlineRegions());
- LOG.info("Before:" + regionsExcludeServer + " After:"
- + cluster.getRegionServer(1).getNumberOfOnlineRegions());
+ try (RegionMover rm = rmBuilder.build()) {
+ rm.unload();
+ LOG.info("Unloading " + rs);
+ assertEquals(0, regionServer.getNumberOfOnlineRegions());
+ assertEquals(regionsExcludeServer, cluster.getRegionServer(1).getNumberOfOnlineRegions());
+ LOG.info("Before:" + regionsExcludeServer + " After:" +
+ cluster.getRegionServer(1).getNumberOfOnlineRegions());
+ }
}
@Test
@@ -243,4 +189,53 @@ public class TestRegionMover {
conf.set(HConstants.REGIONSERVER_PORT, originalPort);
}
}
+
+ /**
+ * UT for HBASE-21746
+ */
+ @Test
+ public void testLoadMetaRegion() throws Exception {
+ HRegionServer rsWithMeta = TEST_UTIL.getMiniHBaseCluster().getRegionServerThreads().stream()
+ .map(t -> t.getRegionServer())
+ .filter(rs -> rs.getRegions(TableName.META_TABLE_NAME).size() > 0).findFirst().get();
+ int onlineRegions = rsWithMeta.getNumberOfOnlineRegions();
+ String rsName = rsWithMeta.getServerName().getAddress().toString();
+ try (RegionMover rm =
+ new RegionMoverBuilder(rsName, TEST_UTIL.getConfiguration()).ack(true).build()) {
+ LOG.info("Unloading " + rsWithMeta.getServerName());
+ rm.unload();
+ assertEquals(0, rsWithMeta.getNumberOfOnlineRegions());
+ LOG.info("Loading " + rsWithMeta.getServerName());
+ rm.load();
+ assertEquals(onlineRegions, rsWithMeta.getNumberOfOnlineRegions());
+ }
+ }
+
+ /**
+ * UT for HBASE-21746
+ */
+ @Test
+ public void testTargetServerDeadWhenLoading() throws Exception {
+ HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0);
+ String rsName = rs.getServerName().getAddress().toString();
+ Configuration conf = new Configuration(TEST_UTIL.getConfiguration());
+ // wait 5 seconds at most
+ conf.setInt(RegionMover.SERVERSTART_WAIT_MAX_KEY, 5);
+ String filename =
+ new Path(TEST_UTIL.getDataTestDir(), "testTargetServerDeadWhenLoading").toString();
+ // unload the region server
+ try (RegionMover rm =
+ new RegionMoverBuilder(rsName, conf).filename(filename).ack(true).build()) {
+ LOG.info("Unloading " + rs.getServerName());
+ rm.unload();
+ assertEquals(0, rs.getNumberOfOnlineRegions());
+ }
+ String inexistRsName = "whatever:123";
+ try (RegionMover rm =
+ new RegionMoverBuilder(inexistRsName, conf).filename(filename).ack(true).build()) {
+ // load the regions to an inexist region server, which should fail and return false
+ LOG.info("Loading to an inexist region server {}", inexistRsName);
+ assertFalse(rm.load());
+ }
+ }
}