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

Reply via email to