saintstack commented on a change in pull request #1639:
URL: https://github.com/apache/hbase/pull/1639#discussion_r419580794
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeAssignmentHelper.java
##########
@@ -600,7 +600,7 @@ protected ServerName getOneRandomServer(String rack,
Set<ServerName> skipServerS
}
if (randomServer != null) {
- return ServerName.valueOf(randomServer.getHostAndPort(),
randomServer.getStartcode());
+ return ServerName.valueOf(randomServer.getAddress(),
randomServer.getStartcode());
Review comment:
Oh, you do it here.
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java
##########
@@ -120,63 +116,24 @@ private ServerName(final Address address, final long
startcode) {
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
+ * @param hostname the hostname string to get the actual hostname from
* @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];
- }
Review comment:
IIRC, this was done to make it so hostnames could be logged w/o the
domain name in an attempt at keeping our log lines shorter and easier to read
##########
File path:
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupUtils.java
##########
@@ -367,9 +367,10 @@ public static String parseHostFromOldLog(Path p) {
String n = p.getName();
int idx = n.lastIndexOf(LOGNAME_SEPARATOR);
String s = URLDecoder.decode(n.substring(0, idx), "UTF8");
- return ServerName.parseHostname(s) + ":" + ServerName.parsePort(s);
+ final ServerName serverName = ServerName.valueOf(s);
+ return serverName.getHostname() + ":" + serverName.getPort();
Review comment:
The ServerName class comment says to do sn.getAddress() if you want to
do something like what is here. What you reckon? I see you get rid of the
getHostAndPort method (good). It too pointed toward sn.getAddress(). What you
think?
Its more a comment for going forward. What is here is fine.
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java
##########
@@ -120,63 +116,24 @@ private ServerName(final Address address, final long
startcode) {
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
+ * @param hostname the hostname string to get the actual hostname from
* @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];
- }
Review comment:
Just FYI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
