hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thejas Nair" <the...@hortonworks.com>
Subject Re: Review Request 25245: Support dynamic service discovery for HiveServer2
Date Wed, 03 Sep 2014 06:56:23 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message