From e00d57195410a6016f7698577965ea5d336dae83 Mon Sep 17 00:00:00 2001 From: Toshihiro Suzuki Date: Sun, 31 Dec 2017 22:29:48 +0900 Subject: [PATCH] HBASE-19572 RegionMover should use the configured default port number and not the one from HConstants --- .../org/apache/hadoop/hbase/util/RegionMover.java | 51 ++++++++++++---------- .../apache/hadoop/hbase/util/TestRegionMover.java | 49 ++++++++++++++------- 2 files changed, 62 insertions(+), 38 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 5e4c8f4..a53309f 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 @@ -101,6 +101,7 @@ public class RegionMover extends AbstractHBaseTool { this.ack = builder.ack; this.port = builder.port; this.timeout = builder.timeout; + setConf(builder.conf); } private RegionMover() { @@ -112,27 +113,47 @@ public class RegionMover extends AbstractHBaseTool { * {@link #ack(boolean)}, {@link #timeout(int)} methods to set the corresponding options */ public static class RegionMoverBuilder { - private boolean ack = true; - private int maxthreads = 1; - private int timeout = Integer.MAX_VALUE; - private String hostname; - private String filename; - private String excludeFile = null; + boolean ack = true; + int maxthreads = 1; + int timeout = Integer.MAX_VALUE; + String hostname; + String filename; + String excludeFile = null; String defaultDir = System.getProperty("java.io.tmpdir"); - private int port = HConstants.DEFAULT_REGIONSERVER_PORT; + final int port; + final Configuration conf; + + public RegionMoverBuilder(String hostname) { + this(hostname, createConf()); + } + + /** + * Creates a new configuration and sets region mover specific overrides + */ + private static Configuration createConf() { + Configuration conf = HBaseConfiguration.create(); + conf.setInt("hbase.client.prefetch.limit", 1); + conf.setInt("hbase.client.pause", 500); + conf.setInt("hbase.client.retries.number", 100); + return conf; + } /** * @param hostname Hostname to unload regions from or load regions to. Can be either hostname * or hostname:port. + * @param conf Configuration object */ - public RegionMoverBuilder(String hostname) { + public RegionMoverBuilder(String hostname, Configuration conf) { String[] splitHostname = hostname.toLowerCase().split(":"); this.hostname = splitHostname[0]; if (splitHostname.length == 2) { this.port = Integer.parseInt(splitHostname[1]); + } else { + this.port = conf.getInt(HConstants.REGIONSERVER_PORT, HConstants.DEFAULT_REGIONSERVER_PORT); } setDefaultfilename(this.hostname); + this.conf = conf; } private void setDefaultfilename(String hostname) { @@ -216,7 +237,6 @@ public class RegionMover extends AbstractHBaseTool { * @throws TimeoutException */ public boolean load() throws ExecutionException, InterruptedException, TimeoutException { - setConf(); ExecutorService loadPool = Executors.newFixedThreadPool(1); Future loadTask = loadPool.submit(new Load(this)); loadPool.shutdown(); @@ -285,7 +305,6 @@ public class RegionMover extends AbstractHBaseTool { * @throws TimeoutException */ public boolean unload() throws InterruptedException, ExecutionException, TimeoutException { - setConf(); deleteFile(this.filename); ExecutorService unloadPool = Executors.newFixedThreadPool(1); Future unloadTask = unloadPool.submit(new Unload(this)); @@ -353,18 +372,6 @@ public class RegionMover extends AbstractHBaseTool { } } - /** - * Creates a new configuration if not already set and sets region mover specific overrides - */ - private void setConf() { - if (conf == null) { - conf = HBaseConfiguration.create(); - conf.setInt("hbase.client.prefetch.limit", 1); - conf.setInt("hbase.client.pause", 500); - conf.setInt("hbase.client.retries.number", 100); - } - } - private void loadRegions(Admin admin, String hostname, int port, List regionsToMove, boolean ack) throws Exception { String server = null; 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 fb9ee84..dd92b95 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 @@ -21,9 +21,11 @@ import static org.junit.Assert.assertEquals; import java.io.File; import java.io.FileWriter; +import org.apache.hadoop.conf.Configuration; 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; @@ -41,6 +43,7 @@ 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 @@ -97,9 +100,9 @@ public class TestRegionMover { int port = regionServer.getServerName().getPort(); int noRegions = regionServer.getNumberOfOnlineRegions(); String rs = rsName + ":" + Integer.toString(port); - RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs).ack(true).maxthreads(8); + RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration()) + .ack(true).maxthreads(8); RegionMover rm = rmBuilder.build(); - rm.setConf(TEST_UTIL.getConfiguration()); LOG.info("Unloading " + rs); rm.unload(); assertEquals(0, regionServer.getNumberOfOnlineRegions()); @@ -119,15 +122,14 @@ public class TestRegionMover { int port = regionServer.getServerName().getPort(); int noRegions = regionServer.getNumberOfOnlineRegions(); String rs = rsName + ":" + Integer.toString(port); - RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs).ack(true); + RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration()) + .ack(true); RegionMover rm = rmBuilder.build(); - rm.setConf(TEST_UTIL.getConfiguration()); LOG.info("Unloading " + rs); rm.unload(); assertEquals(0, regionServer.getNumberOfOnlineRegions()); LOG.info("Successfully Unloaded\nNow Loading"); rm = rmBuilder.ack(false).build(); - rm.setConf(TEST_UTIL.getConfiguration()); rm.load(); TEST_UTIL.waitFor(5000, 500, new Predicate() { @Override @@ -145,9 +147,9 @@ public class TestRegionMover { String rsName = regionServer.getServerName().getHostname(); int port = regionServer.getServerName().getPort(); String rs = rsName + ":" + Integer.toString(port); - RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs).ack(false); + RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration()) + .ack(false); RegionMover rm = rmBuilder.build(); - rm.setConf(TEST_UTIL.getConfiguration()); LOG.info("Unloading " + rs); rm.unload(); TEST_UTIL.waitFor(5000, 500, new Predicate() { @@ -165,9 +167,9 @@ public class TestRegionMover { String rsName = regionServer.getServerName().getHostname(); int port = regionServer.getServerName().getPort(); String rs = rsName + ":" + Integer.toString(port); - RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs).ack(true); + RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration()) + .ack(true); RegionMover rm = rmBuilder.build(); - rm.setConf(TEST_UTIL.getConfiguration()); rm.unload(); LOG.info("Unloading " + rs); assertEquals(0, regionServer.getNumberOfOnlineRegions()); @@ -183,14 +185,13 @@ public class TestRegionMover { String rsName = regionServer.getServerName().getHostname(); int port = regionServer.getServerName().getPort(); String rs = rsName + ":" + Integer.toString(port); - RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs).ack(true); + RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration()) + .ack(true); RegionMover rm = rmBuilder.build(); - rm.setConf(TEST_UTIL.getConfiguration()); rm.unload(); assertEquals(0, regionServer.getNumberOfOnlineRegions()); - rmBuilder = new RegionMoverBuilder(rs).ack(true); + rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration()).ack(true); rm = rmBuilder.build(); - rm.setConf(TEST_UTIL.getConfiguration()); rm.load(); rm.load(); //Repeat the same load. It should be very fast because all regions are already moved. } @@ -215,10 +216,9 @@ public class TestRegionMover { String rsName = regionServer.getServerName().getHostname(); int port = regionServer.getServerName().getPort(); String rs = rsName + ":" + Integer.toString(port); - RegionMoverBuilder rmBuilder = - new RegionMoverBuilder(rs).ack(true).excludeFile(excludeFile.getCanonicalPath()); + RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rs, TEST_UTIL.getConfiguration()) + .ack(true).excludeFile(excludeFile.getCanonicalPath()); RegionMover rm = rmBuilder.build(); - rm.setConf(TEST_UTIL.getConfiguration()); rm.unload(); LOG.info("Unloading " + rs); assertEquals(0, regionServer.getNumberOfOnlineRegions()); @@ -226,4 +226,21 @@ public class TestRegionMover { LOG.info("Before:" + regionsExcludeServer + " After:" + cluster.getRegionServer(1).getNumberOfOnlineRegions()); } + + @Test + public void testRegionServerPort() { + MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster(); + HRegionServer regionServer = cluster.getRegionServer(0); + String rsName = regionServer.getServerName().getHostname(); + + final int PORT = 16021; + Configuration conf = TEST_UTIL.getConfiguration(); + String originalPort = conf.get(HConstants.REGIONSERVER_PORT); + conf.set(HConstants.REGIONSERVER_PORT, Integer.toString(PORT)); + RegionMoverBuilder rmBuilder = new RegionMoverBuilder(rsName, conf); + assertEquals(PORT, rmBuilder.port); + if (originalPort != null) { + conf.set(HConstants.REGIONSERVER_PORT, originalPort); + } + } } -- 2.10.1 (Apple Git-78)