ayushtkn commented on code in PR #5729: URL: https://github.com/apache/hive/pull/5729#discussion_r2028234260
########## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java: ########## @@ -85,7 +85,9 @@ public HiveLockObjectData(String data) { lockMode = elem[2]; queryStr = StringInternUtils.internIfNotNull(elem[3]); if (elem.length >= 5) { - clientIp = elem[4]; Review Comment: if we just do this above with split, will this work automatically? ``` String[] elem = data.split(":", 5); ``` if this works we can avoid the prefix calculation & stuff like that ########## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java: ########## @@ -85,7 +85,9 @@ public HiveLockObjectData(String data) { lockMode = elem[2]; queryStr = StringInternUtils.internIfNotNull(elem[3]); if (elem.length >= 5) { - clientIp = elem[4]; + String prefix = queryId + ":" + lockTime + ":" + lockMode + ":" + queryStr + ":"; + // The client IP is the suffix after the 4th colon, this can be IPv6 with colons + clientIp = data.substring(data.indexOf(prefix) + prefix.length()); Review Comment: ``` clientIp = data.substring(prefix.length()); ``` `data.indexOf(prefix) ` this would be 0 always, If I am catching it right ########## service/src/java/org/apache/hive/service/server/HS2ActivePassiveHARegistry.java: ########## @@ -173,9 +173,9 @@ private void addEndpointToServiceRecord( private void updateEndpoint(final ServiceRecord srv, final String endpointName) { final String instanceUri = srv.get(INSTANCE_URI_CONFIG); - final String[] tokens = instanceUri.split(":"); - final String hostname = tokens[0]; - final int port = Integer.parseInt(tokens[1]); + IPStackUtils.HostPort hostPort = IPStackUtils.splitHostPort(instanceUri); + final String hostname = hostPort.getHostname(); + final int port = hostPort.getPort(); Review Comment: Does something like this work? ``` final URI instanceUri = new URI("//" + srv.get(INSTANCE_URI_CONFIG)); ; final String hostname = instanceUri.getHost(); final int port = instanceUri.getPort(); ``` ########## jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java: ########## @@ -178,12 +179,13 @@ private static void updateParamsWithZKServerNode(JdbcConnectionParams connParams // it must be the server uri added by an older version HS2 Matcher matcher = kvPattern.matcher(dataStr); if ((dataStr != null) && (!matcher.find())) { - String[] split = dataStr.split(":"); - if (split.length != 2) { + try { + IPStackUtils.HostPort hostPort = IPStackUtils.splitHostPort(dataStr); + connParams.setHost(hostPort.getHostname()); + connParams.setPort(hostPort.getPort()); + } catch (Exception e) { throw new ZooKeeperHiveClientException("Unable to parse HiveServer2 URI from ZooKeeper data: " + dataStr); } - connParams.setHost(split[0]); - connParams.setPort(Integer.parseInt(split[1])); Review Comment: I think there is already a Util if the URL thing with // doesn't work, can you check once ``` HostAndPort hostAndPort = HostAndPort.fromString(instanceUri); String hostname = hostAndPort.getHost(); int port = hostAndPort.getPort(); ``` -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org