Skip to content

Commit

Permalink
HBASE-19572 RegionMover should use the configured default port number…
Browse files Browse the repository at this point in the history 
… and not the one from HConstants

Signed-off-by: Reid Chan <reidchan@apache.org>
  • Loading branch information
@brfrn169 @Reidddddd
brfrn169 authored and Reidddddd committed Jul 13, 2018
1 parent e9fdcff commit ce82fd0
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 40 deletions.
52 changes: 28 additions & 24 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
import org.apache.hbase.thirdparty.org.apache.commons.cli.CommandLine;

/**
Expand Down Expand Up @@ -101,6 +102,7 @@ private RegionMover(RegionMoverBuilder builder) {
this.ack = builder.ack;
this.port = builder.port;
this.timeout = builder.timeout;
setConf(builder.conf);
}

private RegionMover() {
Expand All @@ -118,27 +120,43 @@ public static class RegionMoverBuilder {
private String hostname;
private String filename;
private String excludeFile = null;
String defaultDir = System.getProperty("java.io.tmpdir");
private String defaultDir = System.getProperty("java.io.tmpdir");

private int port = HConstants.DEFAULT_REGIONSERVER_PORT;
@VisibleForTesting
final int port;
private 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);
}

private void setDefaultfilename(String hostname) {
this.filename =
defaultDir + File.separator + System.getProperty("user.name") + this.hostname + ":"
+ Integer.toString(this.port);
this.filename = defaultDir + File.separator + System.getProperty("user.name") + this.hostname
+ ":" + Integer.toString(this.port);
this.conf = conf;
}

/**
Expand Down Expand Up @@ -216,7 +234,6 @@ public RegionMover build() {
* @throws TimeoutException
*/
public boolean load() throws ExecutionException, InterruptedException, TimeoutException {
setConf();
ExecutorService loadPool = Executors.newFixedThreadPool(1);
Future<Boolean> loadTask = loadPool.submit(new Load(this));
loadPool.shutdown();
Expand Down Expand Up @@ -285,7 +302,6 @@ public Boolean call() throws IOException {
* @throws TimeoutException
*/
public boolean unload() throws InterruptedException, ExecutionException, TimeoutException {
setConf();
deleteFile(this.filename);
ExecutorService unloadPool = Executors.newFixedThreadPool(1);
Future<Boolean> unloadTask = unloadPool.submit(new Unload(this));
Expand Down Expand Up @@ -353,18 +369,6 @@ public Boolean call() throws IOException {
}
}

/**
* 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<RegionInfo> regionsToMove, boolean ack) throws Exception {
String server = null;
Expand Down
49 changes: 33 additions & 16 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@

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;
Expand All @@ -41,6 +43,7 @@
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
Expand Down Expand Up @@ -97,9 +100,9 @@ public void testLoadWithAck() throws Exception {
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());
Expand All @@ -119,15 +122,14 @@ public void testLoadWithoutAck() throws Exception {
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<Exception>() {
@Override
Expand All @@ -145,9 +147,9 @@ public void testUnloadWithoutAck() throws Exception {
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<Exception>() {
Expand All @@ -165,9 +167,9 @@ public void testUnloadWithAck() throws Exception {
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());
Expand All @@ -183,14 +185,13 @@ public void testRepeatedLoad() throws Exception {
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.
}
Expand All @@ -215,15 +216,31 @@ public void testExclude() throws Exception {
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());
assertEquals(regionsExcludeServer, cluster.getRegionServer(1).getNumberOfOnlineRegions());
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);
}
}
}

0 comments on commit ce82fd0

Please sign in to comment.