----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25245/#review52116 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/25245/#comment90858> zk_ seems rendundant for location in zookeeper jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java <https://reviews.apache.org/r/25245/#comment90880> As this is a param in hiveserver2 jdbc URL, I think "hive.server2." part is redundant. That part makes the url unncessarily verbose. I realize we have two params which have this prefix, but I think we should remove it from those as well (in another jira). jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java <https://reviews.apache.org/r/25245/#comment90881> I think we are likely to have people wanting to implement other modes of dynamically picking the HS2 host. For example, you could simply have multiple HS2 hostnames in a URL (instead of zookeeper hosts). Or people might decide to store the hostnames in another place instead of zookeeper. So I think instead of making this param a boolean, it is better to have the value as "none" (default) or "zookeeper". Maybe change the param name also to "service.discovery.mode" ? jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java <https://reviews.apache.org/r/25245/#comment90859> comment moved to wrong line ? jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java <https://reviews.apache.org/r/25245/#comment90883> I think we can still make use of the java URI class for parameter parsing by just parsing the hostname portion first. Custom parsing of params in this mode can introduce bugs or inconsistencies. The JdbcConnectionParams can be expanded to give a list of hosts. The Utils.parseURL can first extract and substitute the multiple hostnames (if any), and then use the regular java URI parsing. We can have the to validate if the current discovery mode supports multiple hosts, after parsing. - Thejas Nair On Sept. 2, 2014, 10:05 a.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25245/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2014, 10:05 a.m.) > > > Review request for hive, Alan Gates, Navis Ryu, Szehon Ho, and Thejas Nair. > > > Bugs: HIVE-7935 > https://issues.apache.org/jira/browse/HIVE-7935 > > > Repository: hive-git > > > Description > ------- > > https://issues.apache.org/jira/browse/HIVE-7935 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7f4afd9 > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 > jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java > 46044d0 > ql/src/java/org/apache/hadoop/hive/ql/util/ZooKeeperHiveHelper.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java > 59294b1 > service/src/java/org/apache/hive/service/cli/CLIService.java 08ed2e7 > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > 21c33bc > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java > bc0a02c > service/src/java/org/apache/hive/service/cli/session/SessionManager.java > d573592 > > service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java > 37b05fc > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java > 027931e > > service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java > c380b69 > service/src/java/org/apache/hive/service/server/HiveServer2.java 0864dfb > > service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java > 66fc1fc > > Diff: https://reviews.apache.org/r/25245/diff/ > > > Testing > ------- > > Manual testing + test cases. > > > Thanks, > > Vaibhav Gumashta > >
