HBASE-17624 Address late review of HBASE-6721, rsgroups feature

Addresses review comments by Sean Busbey and Appy that happened
to come in long after the commit of HBASE-6721, the original
rsgroup issue.

Also includes subsequent accommodation of Duo Zhang review.

Adds a new type to hold hostname and port. It is called
Address. It is a facade over Guava's HostAndPort. Replace
all instances of HostAndPort with Address. In particular,
those places where HostAndPort was part of the rsgroup
public API.

Fix licenses. Add audience annotations.

Cleanup and note concurrency expectation on a few core classes.
In particular, all access on RSGroupInfoManager is made
synchronized.

M hbase-client/src/main/java/org/apache/hadoop/hbase/ServerName.java
 Host the hostname and port in an instance of the new type Address.
 Add a bunch of deprecation of exotic string parses that should never
 have been public.

M hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java
 Make this an Interface rather than abstract class. Creation was a
 static internal method that only chose one type.... Let it be free
 as a true Interface instead.

hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

@@ -41,7 +41,6 @@
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.Tag;
 import org.apache.hadoop.hbase.TagUtil;
-import org.apache.hadoop.hbase.protobuf.generated.ZooKeeperProtos;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.client.Append;
 import org.apache.hadoop.hbase.client.Consistency;
@@ -79,6 +78,7 @@
 import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.RegionSpecifier;
 import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.RegionSpecifier.RegionSpecifierType;
 import org.apache.hadoop.hbase.protobuf.generated.MapReduceProtos;
+import org.apache.hadoop.hbase.protobuf.generated.ZooKeeperProtos;
 import org.apache.hadoop.hbase.util.Addressing;
 import org.apache.hadoop.hbase.util.ByteStringer;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -1839,4 +1839,4 @@ public static ServerName toServerName(final byte [] data) throws Deserialization
     int port = Addressing.parsePort(str);
     return ServerName.valueOf(hostname, port, -1L);
   }
-}
+}

hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java

@@ -93,9 +93,9 @@ public class TestAsyncProcess {
   private static final byte[] FAILS = "FAILS".getBytes();
   private static final Configuration CONF = new Configuration();
   private static final ConnectionConfiguration CONNECTION_CONFIG = new ConnectionConfiguration(CONF);
-  private static final ServerName sn = ServerName.valueOf("s1:1,1");
-  private static final ServerName sn2 = ServerName.valueOf("s2:2,2");
-  private static final ServerName sn3 = ServerName.valueOf("s3:3,3");
+  private static final ServerName sn = ServerName.valueOf("s1,1,1");
+  private static final ServerName sn2 = ServerName.valueOf("s2,2,2");
+  private static final ServerName sn3 = ServerName.valueOf("s3,3,3");
   private static final HRegionInfo hri1 =
       new HRegionInfo(DUMMY_TABLE, DUMMY_BYTES_1, DUMMY_BYTES_2, false, 1);
   private static final HRegionInfo hri2 =

hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestSimpleRequestController.java

@@ -54,8 +54,8 @@ public class TestSimpleRequestController {
   private static final byte[] DUMMY_BYTES_1 = "DUMMY_BYTES_1".getBytes();
   private static final byte[] DUMMY_BYTES_2 = "DUMMY_BYTES_2".getBytes();
   private static final byte[] DUMMY_BYTES_3 = "DUMMY_BYTES_3".getBytes();
-  private static final ServerName SN = ServerName.valueOf("s1:1,1");
-  private static final ServerName SN2 = ServerName.valueOf("s2:2,2");
+  private static final ServerName SN = ServerName.valueOf("s1,1,1");
+  private static final ServerName SN2 = ServerName.valueOf("s2,2,2");
   private static final HRegionInfo HRI1
           = new HRegionInfo(DUMMY_TABLE, DUMMY_BYTES_1, DUMMY_BYTES_2, false, 1);
   private static final HRegionInfo HRI2

hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java

@@ -29,13 +29,15 @@
 import org.apache.hadoop.hbase.util.Addressing;
 import org.apache.hadoop.hbase.util.Bytes;
 
-import com.google.common.net.HostAndPort;
 import com.google.common.net.InetAddresses;
 
+import org.apache.hadoop.hbase.util.Address;
+
+
 /**
- * Instance of an HBase ServerName.
- * A server name is used uniquely identifying a server instance in a cluster and is made
- * of the combination of hostname, port, and startcode.  The startcode distingushes restarted
+ * Name of a particular incarnation of an HBase Server.
+ * A {@link ServerName} is used uniquely identifying a server instance in a cluster and is made
+ * of the combination of hostname, port, and startcode.  The startcode distinguishes restarted
  * servers on same hostname and port (startcode is usually timestamp of server startup). The
  * {@link #toString()} format of ServerName is safe to use in the  filesystem and as znode name
  * up in ZooKeeper.  Its format is:
@@ -44,15 +46,19 @@
  * For example, if hostname is <code>www.example.org</code>, port is <code>1234</code>,
  * and the startcode for the regionserver is <code>1212121212</code>, then
  * the {@link #toString()} would be <code>www.example.org,1234,1212121212</code>.
- * 
+ *
  * <p>You can obtain a versioned serialized form of this class by calling
- * {@link #getVersionedBytes()}.  To deserialize, call {@link #parseVersionedServerName(byte[])}
- * 
+ * {@link #getVersionedBytes()}.  To deserialize, call
+ * {@link #parseVersionedServerName(byte[])}.
+ *
+ * <p>Use {@link #getAddress()} to obtain the Server hostname + port
+ * (Endpoint/Socket Address).
+ *
  * <p>Immutable.
  */
 @InterfaceAudience.Public
 @InterfaceStability.Evolving
-  public class ServerName implements Comparable<ServerName>, Serializable {
+public class ServerName implements Comparable<ServerName>, Serializable {
   private static final long serialVersionUID = 1367463982557264981L;
 
   /**
@@ -86,10 +92,8 @@ public class ServerName implements Comparable<ServerName>, Serializable {
   public static final String UNKNOWN_SERVERNAME = "#unknown#";
 
   private final String servername;
-  private final String hostnameOnly;
-  private final int port;
   private final long startcode;
-  private transient HostAndPort hostAndPort;
+  private transient Address address;
 
   /**
    * Cached versioned bytes of this ServerName instance.
@@ -99,35 +103,45 @@ public class ServerName implements Comparable<ServerName>, Serializable {
   public static final List<ServerName> EMPTY_SERVER_LIST = new ArrayList<ServerName>(0);
 
   protected ServerName(final String hostname, final int port, final long startcode) {
-    // Drop the domain is there is one; no need of it in a local cluster.  With it, we get long
-    // unwieldy names.
-    this.hostnameOnly = hostname;
-    this.port = port;
+    this(Address.fromParts(hostname, port), startcode);
+  }
+
+  private ServerName(final Address address, final long startcode) {
+    // Use HostAndPort to host port and hostname. Does validation and can do ipv6
+    this.address = address;
     this.startcode = startcode;
-    this.servername = getServerName(hostname, port, startcode);
+    this.servername = getServerName(this.address.getHostname(),
+        this.address.getPort(), startcode);
+  }
+
+  private ServerName(final String serverName) {
+    this(parseHostname(serverName), parsePort(serverName),
+      parseStartcode(serverName));
+  }
+
+  private ServerName(final String hostAndPort, final long startCode) {
+    this(Address.fromString(hostAndPort), startCode);
   }
 
   /**
    * @param hostname
    * @return hostname minus the domain, if there is one (will do pass-through on ip addresses)
+   * @deprecated Since 2.0. This is for internal use only.
    */
+  @Deprecated
+  // Make this private in hbase-3.0.
   static String getHostNameMinusDomain(final String hostname) {
     if (InetAddresses.isInetAddress(hostname)) return hostname;
     String [] parts = hostname.split("\\.");
     if (parts == null || parts.length == 0) return hostname;
     return parts[0];
   }
 
-  private ServerName(final String serverName) {
-    this(parseHostname(serverName), parsePort(serverName),
-      parseStartcode(serverName));
-  }
-
-  private ServerName(final String hostAndPort, final long startCode) {
-    this(Addressing.parseHostname(hostAndPort),
-      Addressing.parsePort(hostAndPort), startCode);
-  }
-
+  /**
+   * @deprecated Since 2.0. Use {@link #valueOf(String)}
+   */
+  @Deprecated
+  // This is unused. Get rid of it.
   public static String parseHostname(final String serverName) {
     if (serverName == null || serverName.length() <= 0) {
       throw new IllegalArgumentException("Passed hostname is null or empty");
@@ -139,11 +153,21 @@ public static String parseHostname(final String serverName) {
     return serverName.substring(0, index);
   }
 
+  /**
+   * @deprecated Since 2.0. Use {@link #valueOf(String)}
+   */
+  @Deprecated
+  // This is unused. Get rid of it.
   public static int parsePort(final String serverName) {
     String [] split = serverName.split(SERVERNAME_SEPARATOR);
     return Integer.parseInt(split[1]);
   }
 
+  /**
+   * @deprecated Since 2.0. Use {@link #valueOf(String)}
+   */
+  @Deprecated
+  // This is unused. Get rid of it.
   public static long parseStartcode(final String serverName) {
     int index = serverName.lastIndexOf(SERVERNAME_SEPARATOR);
     return Long.parseLong(serverName.substring(index + 1));
@@ -189,7 +213,8 @@ public String toString() {
    */
   public String toShortString() {
     return Addressing.createHostAndPortStr(
-        getHostNameMinusDomain(hostnameOnly), port);
+        getHostNameMinusDomain(this.address.getHostname()),
+        this.address.getPort());
   }
 
   /**
@@ -208,11 +233,11 @@ public String getServerName() {
   }
 
   public String getHostname() {
-    return hostnameOnly;
+    return this.address.getHostname();
   }
 
   public int getPort() {
-    return port;
+    return this.address.getPort();
   }
 
   public long getStartcode() {
@@ -226,7 +251,10 @@ public long getStartcode() {
    * @param startcode
    * @return Server name made of the concatenation of hostname, port and
    * startcode formatted as <code>&lt;hostname&gt; ',' &lt;port&gt; ',' &lt;startcode&gt;</code>
+   * @deprecated Since 2.0. Use {@link ServerName#valueOf(String, int, long)} instead.
    */
+  @Deprecated
+  // TODO: Make this private in hbase-3.0.
   static String getServerName(String hostName, int port, long startcode) {
     final StringBuilder name = new StringBuilder(hostName.length() + 1 + 5 + 1 + 13);
     name.append(hostName.toLowerCase(Locale.ROOT));
@@ -242,7 +270,9 @@ static String getServerName(String hostName, int port, long startcode) {
    * @param startcode
    * @return Server name made of the concatenation of hostname, port and
    * startcode formatted as <code>&lt;hostname&gt; ',' &lt;port&gt; ',' &lt;startcode&gt;</code>
+   * @deprecated Since 2.0. Use {@link ServerName#valueOf(String, long)} instead.
    */
+  @Deprecated
   public static String getServerName(final String hostAndPort,
       final long startcode) {
     int index = hostAndPort.indexOf(":");
@@ -254,22 +284,23 @@ public static String getServerName(final String hostAndPort,
   /**
    * @return Hostname and port formatted as described at
    * {@link Addressing#createHostAndPortStr(String, int)}
+   * @deprecated Since 2.0. Use {@link #getAddress()} instead.
    */
+  @Deprecated
   public String getHostAndPort() {
-    return Addressing.createHostAndPortStr(hostnameOnly, port);
+    return this.address.toString();
   }
 
-  public HostAndPort getHostPort() {
-    if (hostAndPort == null) {
-      hostAndPort = HostAndPort.fromParts(hostnameOnly, port);
-    }
-    return hostAndPort;
+  public Address getAddress() {
+    return this.address;
   }
 
   /**
    * @param serverName ServerName in form specified by {@link #getServerName()}
    * @return The server start code parsed from <code>servername</code>
+   * @deprecated Since 2.0. Use instance of ServerName to pull out start code.
    */
+  @Deprecated
   public static long getServerStartcodeFromServerName(final String serverName) {
     int index = serverName.lastIndexOf(SERVERNAME_SEPARATOR);
     return Long.parseLong(serverName.substring(index + 1));
@@ -279,7 +310,9 @@ public static long getServerStartcodeFromServerName(final String serverName) {
    * Utility method to excise the start code from a server name
    * @param inServerName full server name
    * @return server name less its start code
+   * @deprecated Since 2.0. Use {@link #getAddress()}
    */
+  @Deprecated
   public static String getServerNameLessStartCode(String inServerName) {
     if (inServerName != null && inServerName.length() > 0) {
       int index = inServerName.lastIndexOf(SERVERNAME_SEPARATOR);
@@ -296,7 +329,6 @@ public int compareTo(ServerName other) {
     if (compare != 0) return compare;
     compare = this.getPort() - other.getPort();
     if (compare != 0) return compare;
-
     return Long.compare(this.getStartcode(), other.getStartcode());
   }
 
@@ -320,6 +352,7 @@ public boolean equals(Object o) {
    */
   public static boolean isSameHostnameAndPort(final ServerName left,
       final ServerName right) {
+    // TODO: Make this left.getAddress().equals(right.getAddress())
     if (left == null) return false;
     if (right == null) return false;
     return left.getHostname().compareToIgnoreCase(right.getHostname()) == 0 &&
@@ -365,4 +398,4 @@ public static boolean isFullServerName(final String str){
     if (str == null ||str.isEmpty()) return false;
     return SERVERNAME_PATTERN.matcher(str).matches();
   }
-}
+}