You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vaibhav Gumashta <vg...@hortonworks.com> on 2014/09/02 12:05:03 UTC

Review Request 25245: Support dynamic service discovery for HiveServer2

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

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


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.

> On Sept. 3, 2014, 7:15 a.m., Thejas Nair wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java, line 84
> > <https://reviews.apache.org/r/25245/diff/1/?file=673807#file673807line84>
> >
> >     How big are the zookeeper jars ? If we use zookeeper in this class, I believe zookeeper jars will always be needed for jdbc driver.
> >     It would be better to have the zookeeper service discovery code in a separate util class. That way we will need zookeeper jars only if this mode is used.

ZooKeeper jars are about 780KB. I was wondering what is a better option:
1. If dynamic service discovery using ZK is supported, add the following jar(s) to your classpath. Or
2. The JDBC jar you get is indifferent to this config - always works.

Personally, I am bending towards a simpler client deployment, otherwise 1.) adds one more knob that needs to be supported/kept updated via documentation. Thoughts?


- Vaibhav


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52139
-----------------------------------------------------------


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


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52139
-----------------------------------------------------------



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
<https://reviews.apache.org/r/25245/#comment90887>

    How big are the zookeeper jars ? If we use zookeeper in this class, I believe zookeeper jars will always be needed for jdbc driver.
    It would be better to have the zookeeper service discovery code in a separate util class. That way we will need zookeeper jars only if this mode is used.



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment90889>

    It will be useful to log at info level that it is re-using the existing znode.



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment90890>

    It will be useful to have HS2 de-register itself if it gets a kill signal it can handle.
    That part can be done as part of follow-up jira as well. (Until then admin will need to manually edit the zookeeper entry).


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


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.

> On Sept. 3, 2014, 6:56 a.m., Thejas Nair wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java, line 142
> > <https://reviews.apache.org/r/25245/diff/1/?file=673807#file673807line142>
> >
> >     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.

Can't convert to URI directly, unless we resolve the authority part in the URI string to a valid value (from host1:port1,host2:port2,host3:port3 to host:port). For the resolution to happen, we need to determine from the URI string whether SERVICE_DISCOVERY_MODE = zooKeeper or not and then get a host from ZooKeeper. But I'll move the logic to Utils#parseURL. Let me know if you think otherwise.


> On Sept. 3, 2014, 6:56 a.m., Thejas Nair wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java, line 110
> > <https://reviews.apache.org/r/25245/diff/1/?file=673807#file673807line110>
> >
> >     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" ?

Actually, I see one more problem here. The jdbc url: jdbc:hive2://<host>:<port>/dbName;sess_var_list?hive_conf_list#hive_var_list contains hive_conf_list & hive_var_list which are used to set the corresponding values on the server side (for this connection) while opening a client session. As you have pointed out earlier, we haven't done a great job of keeping "session only" configs in the sess_var_list (e.g by specifying hive.server2.thrift.http.path, the client actually is trying to point to the corresponding path for the server, and not trying to set its value). I'll create a follow up jira to do that cleanup. And I agree, we can keep variable names short in sess_var_list and probably use a camelCase convention to keep the intent clear.


- Vaibhav


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52116
-----------------------------------------------------------


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


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Alan Gates <ga...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52171
-----------------------------------------------------------



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment90921>

    It seems like we want more than warn here if we fail to create the parent node.  In this case we'll be unable to create the node for this instance, and clients will be unable to find the server.  I would think this should be fatal.



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment90922>

    Agree we should have a clean shutdown case.  The timeout was 3 minutes I think, which means it will be a while after the system shuts down that clients keep trying to contact it.


- Alan Gates


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


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.

> On Sept. 11, 2014, 7:24 p.m., Thejas Nair wrote:
> > service/src/java/org/apache/hive/service/server/HiveServer2.java, line 114
> > <https://reviews.apache.org/r/25245/diff/4-5/?file=684777#file684777line114>
> >
> >     Should we do a 'mkdir -p' equivalent as done in ZookeeperTokenStore.ensurePath ?

Yup, that's a cleaner approach. Adding it for the namespace path though as server znode will never have path components.


- Vaibhav


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53095
-----------------------------------------------------------


On Sept. 11, 2014, 1:08 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 1:08 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53095
-----------------------------------------------------------



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment92493>

    Should we do a 'mkdir -p' equivalent as done in ZookeeperTokenStore.ensurePath ?



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment92494>

    I think this is an important message about shut down, lets log it at WARN level.


- Thejas Nair


On Sept. 11, 2014, 1:08 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 1:08 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53415
-----------------------------------------------------------

Ship it!


Ship It!

- Thejas Nair


On Sept. 15, 2014, 9:19 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2014, 9:19 p.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 cfb3f9a 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 15, 2014, 9:19 p.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 (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java cfb3f9a 
  itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
  jdbc/pom.xml 1ad13a7 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53387
-----------------------------------------------------------



itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java
<https://reviews.apache.org/r/25245/#comment93033>

    Looks like we have a backward incompatible change here. There is a new requirement that remote mode has to end with "/" .


- Thejas Nair


On Sept. 15, 2014, 12:08 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2014, 12:08 p.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 cfb3f9a 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 15, 2014, 12:08 p.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 (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java cfb3f9a 
  itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
  jdbc/pom.xml 1ad13a7 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 15, 2014, 5:18 a.m.)


Review request for hive, Alan Gates, Navis Ryu, Szehon Ho, and Thejas Nair.


Changes
-------

Fixes unit test failures.


Bugs: HIVE-7935
    https://issues.apache.org/jira/browse/HIVE-7935


Repository: hive-git


Description
-------

https://issues.apache.org/jira/browse/HIVE-7935


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java cfb3f9a 
  itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java e1d44ec 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
  jdbc/pom.xml 1ad13a7 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53205
-----------------------------------------------------------



service/src/java/org/apache/hive/service/cli/session/SessionManager.java
<https://reviews.apache.org/r/25245/#comment92704>

    synchronization of this function does not seem to be needed. handToSession is a ConcurrentHashMap, it establishes happens-before relationship between update and access.


- Thejas Nair


On Sept. 12, 2014, 12:30 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2014, 12:30 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 12, 2014, 12:30 p.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 (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5d2e6b0 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
  jdbc/pom.xml 1ad13a7 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 12, 2014, 1:25 a.m.)


Review request for hive, Alan Gates, Navis Ryu, Szehon Ho, and Thejas Nair.


Changes
-------

All the feedback changes except changing the ACL on znodes (I'm  figuring out the best solution).


Bugs: HIVE-7935
    https://issues.apache.org/jira/browse/HIVE-7935


Repository: hive-git


Description
-------

https://issues.apache.org/jira/browse/HIVE-7935


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5d2e6b0 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
  jdbc/pom.xml 1ad13a7 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53112
-----------------------------------------------------------



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/25245/#comment92525>

    1. Read/write locks - when hive.lock.manager is set to  org.apache.hadoop.hive.ql.lockmgr.zookeeper.ZooKeeperHiveLockManager


- Thejas Nair


On Sept. 11, 2014, 1:08 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 1:08 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53029
-----------------------------------------------------------



service/src/java/org/apache/hive/service/cli/CLIService.java
<https://reviews.apache.org/r/25245/#comment92378>

    I'll get rid of this import - leftover from different things I was trying.



service/src/java/org/apache/hive/service/cli/CLIService.java
<https://reviews.apache.org/r/25245/#comment92379>

    Will get rid of this too.


- Vaibhav Gumashta


On Sept. 11, 2014, 1:08 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 1:08 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.

> On Sept. 11, 2014, 6:33 p.m., Thejas Nair wrote:
> > service/src/java/org/apache/hive/service/server/HiveServer2.java, line 129
> > <https://reviews.apache.org/r/25245/diff/4-5/?file=684777#file684777line129>
> >
> >     Should we use READ_ACL_UNSAFE here ?

Sorry I meant to use that. But as you mention in the comment below, we'll need to construct an ACL which gives write/delete/create to a specific user id and read to all.


> On Sept. 11, 2014, 6:33 p.m., Thejas Nair wrote:
> > service/src/java/org/apache/hive/service/server/HiveServer2.java, line 146
> > <https://reviews.apache.org/r/25245/diff/4-5/?file=684777#file684777line146>
> >
> >     I think we should check here as well, if it is OK to shut down the server.
> >     Consider the case of a rolling upgrade late in the night, when the cluster might not be very active. There might not be any remaining active connections on this server. And since it is removed from the zookeeper, no new connections would be established, and server would not come down by itself.

Good point, will make the change.


- Vaibhav


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53067
-----------------------------------------------------------


On Sept. 11, 2014, 1:08 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 1:08 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53067
-----------------------------------------------------------



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment92456>

    Should we use READ_ACL_UNSAFE here ?



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment92467>

    Its safer to first set the boolean and then set the watcher, to avoid potential race conditions. Otherwise, in theory you can have a case where node gets deleted, watcher sets boolean to false, and then this line sets it to true.
    
    Also, the "this." for the function call seems unnecessary.
    
    We should also check if the exists call returns null (which indicates that file does not exist). call stop in that case ?



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment92474>

    I think we should check here as well, if it is OK to shut down the server.
    Consider the case of a rolling upgrade late in the night, when the cluster might not be very active. There might not be any remaining active connections on this server. And since it is removed from the zookeeper, no new connections would be established, and server would not come down by itself.


- Thejas Nair


On Sept. 11, 2014, 1:08 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 1:08 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review53097
-----------------------------------------------------------



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment92500>

    I read some more about zookeeper ACL. Looks like just this READ_ACL_UNSAFE will not allow the hive user to delete the entry.
    It seems like we need to combine CREATOR_ALL_ACL and READ_ACL_UNSAFE and pass on to these functions.
    But looking at CREATOR_ALL_ACL code in zookeeper and the definition of "auth" scheme, it seems like it considers any authenticated user as owner, not just the current user. We need to clarify this with some zookeeper export.


- Thejas Nair


On Sept. 11, 2014, 1:08 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 1:08 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 11, 2014, 1:08 p.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 (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5d2e6b0 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
  jdbc/pom.xml 1ad13a7 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52990
-----------------------------------------------------------



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment92297>

    We can restrict to be writable by only hive.
    Admins would be able to sudo as hive or login using its keytab.


- Thejas Nair


On Sept. 10, 2014, 9:43 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2014, 9:43 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 10, 2014, 9:43 p.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 (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5d2e6b0 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
  jdbc/pom.xml 1ad13a7 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.

> On Sept. 8, 2014, 10:05 p.m., Thejas Nair wrote:
> > service/src/java/org/apache/hive/service/server/HiveServer2.java, line 111
> > <https://reviews.apache.org/r/25245/diff/1-3/?file=673819#file673819line111>
> >
> >     Does Ids.OPEN_ACL_UNSAFE allow anyone can delete this entry ?

Yes, currently the admin will be going through the ZK cli - I'm not sure if we can be too restrictive there.


- Vaibhav


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52637
-----------------------------------------------------------


On Sept. 10, 2014, 9:43 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2014, 9:43 p.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 5d2e6b0 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/pom.xml 1ad13a7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52637
-----------------------------------------------------------



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment91510>

    Does Ids.OPEN_ACL_UNSAFE allow anyone can delete this entry ?



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment91513>

    should this be error and not fatal ? Fatal usually indicates that you are going to exit the program.
    
    https://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/Level.html#FATAL



service/src/java/org/apache/hive/service/server/HiveServer2.java
<https://reviews.apache.org/r/25245/#comment91514>

    Error log level seems more appropriate here.


- Thejas Nair


On Sept. 8, 2014, 7:43 a.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 7:43 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 31aeba9 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52633
-----------------------------------------------------------


- Thejas Nair


On Sept. 8, 2014, 7:43 a.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 7:43 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 31aeba9 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.

> On Sept. 8, 2014, 8:19 p.m., Thejas Nair wrote:
> > jdbc/src/java/org/apache/hive/jdbc/Utils.java, line 353
> > <https://reviews.apache.org/r/25245/diff/3/?file=682449#file682449line353>
> >
> >     You can just just SERVICE_DISCOVERY_MODE_ZOOKEEPER instead of JdbcConnectionParams.SERVICE_DISCOVERY_MODE_ZOOKEEPER

never mind this one, the classname is needed as its used from different class.


- Thejas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52618
-----------------------------------------------------------


On Sept. 8, 2014, 7:43 a.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 7:43 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 31aeba9 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52618
-----------------------------------------------------------



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
<https://reviews.apache.org/r/25245/#comment91476>

    It will be simpler and less code if we assume that we support one or more hostnames in the URL.
    
    First we extract the one or more hostnames - ie, what extractZooKeeperEnsemble is doing.
    Then store the hostname or list of hostnames in a variable, and replace the host in uri with a dummy hostname that is acceptable for URI parsing.
    
    After that we can extract variable names without additonal logic. You can then check if multiple hostnames are expected based on the value of SERVICE_DISCOVERY_MODE



jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java
<https://reviews.apache.org/r/25245/#comment91468>

    how about extending SQLException, so that you don't have to wrap it elsewhere ?



jdbc/src/java/org/apache/hive/jdbc/Utils.java
<https://reviews.apache.org/r/25245/#comment91471>

    Lets use jiras for tracking planed changes instead of TODOs in code.



jdbc/src/java/org/apache/hive/jdbc/Utils.java
<https://reviews.apache.org/r/25245/#comment91470>

    I don't think we need these TODOs here. We are already tracking them in Jiras.



jdbc/src/java/org/apache/hive/jdbc/Utils.java
<https://reviews.apache.org/r/25245/#comment91473>

    the old variable name jdbcURI is reasonable/good, as URI is an acryonym. Keeping the old name will avoid unnecessary diffs.



jdbc/src/java/org/apache/hive/jdbc/Utils.java
<https://reviews.apache.org/r/25245/#comment91474>

    You can just just SERVICE_DISCOVERY_MODE_ZOOKEEPER instead of JdbcConnectionParams.SERVICE_DISCOVERY_MODE_ZOOKEEPER


- Thejas Nair


On Sept. 8, 2014, 7:43 a.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25245/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 7:43 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 31aeba9 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
>   jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
>   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 a0bc905 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
>   service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 
> 
> Diff: https://reviews.apache.org/r/25245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 8, 2014, 7:43 a.m.)


Review request for hive, Alan Gates, Navis Ryu, Szehon Ho, and Thejas Nair.


Changes
-------

Patch rebased on trunk.


Bugs: HIVE-7935
    https://issues.apache.org/jira/browse/HIVE-7935


Repository: hive-git


Description
-------

https://issues.apache.org/jira/browse/HIVE-7935


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 31aeba9 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java ae128a9 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java b0bb8be 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 7, 2014, 1:35 p.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 a0a5f54 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 0b5ef12 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java 66fc1fc 

Diff: https://reviews.apache.org/r/25245/diff/


Testing (updated)
-------

Manual testing.


Thanks,

Vaibhav Gumashta


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/
-----------------------------------------------------------

(Updated Sept. 7, 2014, 1:29 p.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 (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a0a5f54 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 
  jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 
  jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java PRE-CREATION 
  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 0919d2f 
  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 a0bc905 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java f5a8f27 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 0b5ef12 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 11d25cc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java 2b80adc 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 443c371 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 4067106 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c 
  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


Re: Review Request 25245: Support dynamic service discovery for HiveServer2

Posted by Lefty Leverenz <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25245/#review52070
-----------------------------------------------------------



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/25245/#comment90803>

    To be user friendly, please use the parameter name (hive.zookeeper.quorum) instead of HIVE_ZOOKEEPER_QUORUM.
    
    And shouldn't it say "The port for" instead of "The port of"?



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/25245/#comment90804>

    Use the parameter name (hive.zookeeper.quorum) instead of HIVE_ZOOKEEPER_QUORUM, unless the connection string can specify it that way.



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/25245/#comment90806>

    This would need to be rebased because HIVE-5799 just changed the default from ms to sec, but it looks like you only got rid of a space at eol -- that's already taken care of, so you don't need this change.


- Lefty Leverenz


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