diff --git a/src/main/java/org/apache/hadoop/hbase/MasterAddressTracker.java b/src/main/java/org/apache/hadoop/hbase/MasterAddressTracker.java index ed8c421..20c0799 100644 --- a/src/main/java/org/apache/hadoop/hbase/MasterAddressTracker.java +++ b/src/main/java/org/apache/hadoop/hbase/MasterAddressTracker.java @@ -58,8 +58,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker { * @return Server name or null if timed out. */ public ServerName getMasterAddress() { - byte [] data = super.getData(); - return data == null ? null : new ServerName(Bytes.toString(data)); + return bytesToServerName(super.getData()); } /** @@ -81,7 +80,14 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker { */ public synchronized ServerName waitForMaster(long timeout) throws InterruptedException { - byte [] data = super.blockUntilAvailable(); - return data == null ? null : new ServerName(Bytes.toString(data)); + return bytesToServerName(super.blockUntilAvailable()); + } + + /** + * @param bytes Byte array of {@link ServerName#toString()} + * @return A {@link ServerName} instance. + */ + private ServerName bytesToServerName(final byte [] bytes) { + return bytes == null ? null : new ServerName(Bytes.toString(bytes)); } } \ No newline at end of file diff --git a/src/main/java/org/apache/hadoop/hbase/ServerName.java b/src/main/java/org/apache/hadoop/hbase/ServerName.java index 973ec1c..3aed06b 100644 --- a/src/main/java/org/apache/hadoop/hbase/ServerName.java +++ b/src/main/java/org/apache/hadoop/hbase/ServerName.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase; import java.util.Collection; +import java.util.regex.Pattern; import org.apache.hadoop.hbase.util.Addressing; import org.apache.hadoop.hbase.util.Bytes; @@ -41,11 +42,21 @@ import org.apache.hadoop.hbase.util.Bytes; */ public class ServerName implements Comparable { /** + * What to use if no startcode supplied. + */ + public static final int NON_STARTCODE = -1; + + /** * This character is used as separator between server hostname, port and * startcode. */ public static final String SERVERNAME_SEPARATOR = ","; + public static Pattern SERVERNAME_PATTERN = + Pattern.compile(Addressing.VALID_HOSTNAME_REGEX_PREFIX + + SERVERNAME_SEPARATOR + Addressing.VALID_PORT_REGEX + + SERVERNAME_SEPARATOR + Addressing.VALID_PORT_REGEX + "$"); + private final String servername; private final String hostname; private final int port; @@ -230,4 +241,16 @@ public class ServerName implements Comparable { return left.getHostname().equals(right.getHostname()) && left.getPort() == right.getPort(); } -} + + /** + * Use this method instead of {@link ServerName#ServerName(String)} if you + * are not sure str is output of + * a {@link ServerName#toString()} or is a "'' ':' ''" combo. + * @param str A {@link ServerName} as String OR a hostname:port combo. + * @return A ServerName instance. + */ + public static ServerName parseServerName(final String str) { + return SERVERNAME_PATTERN.matcher(str).matches()? + new ServerName(str): new ServerName(str, NON_STARTCODE); + } +} \ No newline at end of file diff --git a/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java b/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java index 695fdf2..32bc68a 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java +++ b/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java @@ -145,7 +145,8 @@ class ActiveMasterManager extends ZooKeeperListener { this.clusterHasActiveMaster.set(true); byte [] bytes = ZKUtil.getDataAndWatch(this.watcher, this.watcher.masterAddressZNode); - ServerName currentMaster = new ServerName(Bytes.toString(bytes)); + ServerName currentMaster = + ServerName.parseServerName(Bytes.toString(bytes)); if (ServerName.isSameHostnameAndPort(currentMaster, this.sn)) { String msg = ("Current master has this master's address, " + currentMaster + "; master was restarted? Waiting on znode to expire..."); diff --git a/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index abfb076..3fb5fe7 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -763,9 +763,8 @@ implements HMasterInterface, HMasterRegionInterface, MasterServices, Server { @Override public void reportRSFatalError(byte [] sn, String errorText) { - ServerName serverName = new ServerName(sn); - String msg = "Region server " + serverName + " reported a fatal error:\n" - + errorText; + String msg = "Region server " + Bytes.toString(sn) + + " reported a fatal error:\n" + errorText; LOG.error(msg); rsFatals.add(msg); } diff --git a/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index b9c850d..9953d3f 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -205,7 +205,8 @@ public class MasterFileSystem { } List serverNames = new ArrayList(); for (FileStatus status : logFolders) { - ServerName serverName = new ServerName(status.getPath().getName()); + ServerName serverName = + ServerName.parseServerName(status.getPath().getName()); if (!onlineServers.contains(serverName)) { LOG.info("Log folder " + status.getPath() + " doesn't belong " + "to a known region server, splitting"); diff --git a/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java b/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java index f994f99..49aca7c 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java +++ b/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java @@ -249,7 +249,7 @@ public class ServerManager { * @deprecated Use {@link #getLoad(HServerAddress)} */ public HServerLoad getLoad(final HServerAddress address) { - ServerName sn = new ServerName(address.toString(), -1); + ServerName sn = new ServerName(address.toString(), ServerName.NON_STARTCODE); ServerName actual = ServerName.findServerWithSameHostnamePort(this.getOnlineServersList(), sn); return actual == null? null: getLoad(actual); diff --git a/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java b/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java index 6ba24c0..6df265d 100644 --- a/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java +++ b/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java @@ -246,7 +246,7 @@ public class ReplicationZookeeper { } List addresses = new ArrayList(children.size()); for (String child : children) { - addresses.add(new ServerName(child)); + addresses.add(ServerName.parseServerName(child)); } return addresses; } diff --git a/src/main/java/org/apache/hadoop/hbase/util/Addressing.java b/src/main/java/org/apache/hadoop/hbase/util/Addressing.java index 714e2d9..4fb7769 100644 --- a/src/main/java/org/apache/hadoop/hbase/util/Addressing.java +++ b/src/main/java/org/apache/hadoop/hbase/util/Addressing.java @@ -25,6 +25,21 @@ import java.net.InetSocketAddress; * Utility for network addresses, resolving and naming. */ public class Addressing { + /** + * Regex for RFC952 hostname matching. Does not have a '$" on the end. Can + * be used as prefix on a larger regex. + * @see http://stackoverflow.com/questions/106179/regular-expression-to-match-hostname-or-ip-address + */ + public static final String VALID_HOSTNAME_REGEX_PREFIX = + "^(([a-zA-Z]|[a-zA-Z][a-zA-Z0-9\\-]*[a-zA-Z0-9])\\.)*([A-Za-z]|[A-Za-z][A-Za-z0-9\\-]*[A-Za-z0-9])"; + /** + * Regex for RFC952 hostname matching. + * @see http://stackoverflow.com/questions/106179/regular-expression-to-match-hostname-or-ip-address + */ + public static final String VALID_HOSTNAME_REGEX = + VALID_HOSTNAME_REGEX_PREFIX + "$"; + + public static final String VALID_PORT_REGEX = "[\\d]+"; public static final String HOSTNAME_PORT_SEPARATOR = ":"; /** diff --git a/src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerTracker.java b/src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerTracker.java index 83a51bd..cb94ddb 100644 --- a/src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerTracker.java +++ b/src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerTracker.java @@ -30,7 +30,6 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.master.ServerManager; -import org.apache.hadoop.hbase.zookeeper.ZKUtil.NodeAndData; import org.apache.zookeeper.KeeperException; /** @@ -75,7 +74,7 @@ public class RegionServerTracker extends ZooKeeperListener { synchronized(this.regionServers) { this.regionServers.clear(); for (String n: servers) { - ServerName sn = new ServerName(ZKUtil.getNodeName(n)); + ServerName sn = ServerName.parseServerName(ZKUtil.getNodeName(n)); this.regionServers.add(sn); } } @@ -93,7 +92,7 @@ public class RegionServerTracker extends ZooKeeperListener { String serverName = ZKUtil.getNodeName(path); LOG.info("RegionServer ephemeral node deleted, processing expiration [" + serverName + "]"); - ServerName sn = new ServerName(serverName); + ServerName sn = ServerName.parseServerName(serverName); if (!serverManager.isServerOnline(sn)) { LOG.info(serverName.toString() + " is not online"); return; diff --git a/src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java b/src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java index 51f7725..663d177 100644 --- a/src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java +++ b/src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java @@ -94,7 +94,7 @@ public class RootRegionTracker extends ZooKeeperNodeTracker { int index = str.indexOf(ServerName.SERVERNAME_SEPARATOR); if (index != -1) { // Presume its ServerName.toString() format. - return new ServerName(str); + return ServerName.parseServerName(str); } // Presume it a hostname:port format. String hostname = Addressing.parseHostname(str); diff --git a/src/test/java/org/apache/hadoop/hbase/TestServerName.java b/src/test/java/org/apache/hadoop/hbase/TestServerName.java index 298fbe6..7304ff8 100644 --- a/src/test/java/org/apache/hadoop/hbase/TestServerName.java +++ b/src/test/java/org/apache/hadoop/hbase/TestServerName.java @@ -19,14 +19,31 @@ */ package org.apache.hadoop.hbase; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertTrue; -import java.util.HashSet; -import java.util.Set; +import java.util.regex.Pattern; +import org.apache.hadoop.hbase.util.Addressing; import org.junit.Test; public class TestServerName { + @Test public void testFormatChecker() { + assertTrue(Pattern.matches(Addressing.VALID_PORT_REGEX, "123")); + assertFalse(Pattern.matches(Addressing.VALID_PORT_REGEX, "")); + assertTrue(Pattern.matches(Addressing.VALID_HOSTNAME_REGEX, "example.org")); + assertTrue(Pattern.matches(Addressing.VALID_HOSTNAME_REGEX, + "www1.example.org")); + assertTrue(ServerName.SERVERNAME_PATTERN.toString(), + ServerName.SERVERNAME_PATTERN.matcher("www1.example.org,1234,567").matches()); + assertEquals("www.example.org,1234,5678", + ServerName.parseServerName("www.example.org,1234,5678").toString()); + assertEquals("www.example.org,1234,0", + ServerName.parseServerName("www.example.org:1234").toString()); + } + @Test public void testServerName() { ServerName sn = new ServerName("www.example.org", 1234, 5678);