Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15036 )
Change subject: [java] support resolve one master address to multiple addresses ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java: http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@57 PS1, Line 57: private List<Pair<InetAddress, HostAndPort>> masterAddrsWithNames; Based on the usage, doesn't seem like this needs to be a member variable. Local variable in method connectToMasters() would suffice, no? http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@203 PS1, Line 203: InetAddress inetAddress = NetUtil.getInetAddress(hostAndPort.getHost()); Curious similarly why not fetch all addresses of each master host like above? http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@214 PS1, Line 214: Deferred<ConnectToMasterResponsePB> d; Nit: Better to combine the definition and assignment of "d" to line 219 below. -- To view, visit http://gerrit.cloudera.org:8080/15036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie242bc1331596902fa16f1c6d1b439d78b73977a Gerrit-Change-Number: 15036 Gerrit-PatchSet: 1 Gerrit-Owner: Yifan Zhang <chinazhangyi...@163.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 19:57:59 +0000 Gerrit-HasComments: Yes
