You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Navis Ryu <na...@nexr.com> on 2013/11/12 07:19:07 UTC

Review Request 15449: session/operation timeout for hiveserver2

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

Review request for hive.


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
  conf/hive-default.xml.template c574ab5 
  service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 6f4b8dc 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 00058cc 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java cfda752 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 25c6f38 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Nov. 14, 2013, 2:43 a.m., Thejas Nair wrote:
> > conf/hive-default.xml.template, line 891
> > <https://reviews.apache.org/r/15449/diff/2/?file=382964#file382964line891>
> >
> >     "Should be a positive value" sounds like 0 will result in an error. I think it will be better to rephrase this as "Timeout is applicable only if the value is positive"
> >

Ok.


> On Nov. 14, 2013, 2:43 a.m., Thejas Nair wrote:
> > service/src/java/org/apache/hive/service/cli/session/SessionManager.java, line 56
> > <https://reviews.apache.org/r/15449/diff/2/?file=382969#file382969line56>
> >
> >     did you mean to use volatile and not transient ?

Right.


> On Nov. 14, 2013, 2:43 a.m., Thejas Nair wrote:
> > service/src/java/org/apache/hive/service/cli/operation/Operation.java, line 85
> > <https://reviews.apache.org/r/15449/diff/2/?file=382966#file382966line85>
> >
> >     I am not sure if we need an operation level timeout, I think the Session timeout will suffice for most use cases.
> >     But since this can be disabled through config value, I think it is OK.
> >

Operations seemed to be leaked sometimes when using JDBC. Just for the case.


> On Nov. 14, 2013, 2:43 a.m., Thejas Nair wrote:
> > service/src/java/org/apache/hive/service/cli/session/SessionManager.java, line 97
> > <https://reviews.apache.org/r/15449/diff/2/?file=382969#file382969line97>
> >
> >     should we put "handleToSession.values()" in a synchronized block as well ?
> >

Right. Quite embarrassing. --a


- Navis


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


On Nov. 18, 2013, 2:32 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2013, 2:32 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
>   conf/hive-default.xml.template c574ab5 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 6f4b8dc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 00058cc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java cfda752 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java e262b72 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

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



conf/hive-default.xml.template
<https://reviews.apache.org/r/15449/#comment55914>

    "Should be a positive value" sounds like 0 will result in an error. I think it will be better to rephrase this as "Timeout is applicable only if the value is positive"
    



service/src/java/org/apache/hive/service/cli/operation/Operation.java
<https://reviews.apache.org/r/15449/#comment55913>

    I am not sure if we need an operation level timeout, I think the Session timeout will suffice for most use cases.
    But since this can be disabled through config value, I think it is OK.
    



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

    did you mean to use volatile and not transient ?



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

    should we put "handleToSession.values()" in a synchronized block as well ?
    


- Thejas Nair


On Nov. 13, 2013, 1:21 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2013, 1:21 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
>   conf/hive-default.xml.template c574ab5 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 6f4b8dc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 00058cc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java cfda752 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 25c6f38 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

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



conf/hive-default.xml.template
<https://reviews.apache.org/r/15449/#comment55916>

    I think the default can be set to a few days. That way we can be reasonably sure that the user won't expect the connection to still be around. Maybe 7 days ?
    7*24*3600*1000.
    
    Also, we can probably set the unit of this as seconds or minutes.
    


- Thejas Nair


On Nov. 13, 2013, 1:21 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2013, 1:21 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
>   conf/hive-default.xml.template c574ab5 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 6f4b8dc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 00058cc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java cfda752 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 25c6f38 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Jan. 22, 2014, 7:02 a.m., Carl Steinbach wrote:
> > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java, line 397
> > <https://reviews.apache.org/r/15449/diff/2/?file=382968#file382968line397>
> >
> >     I think the name of the method should accurately reflect the purpose it currently serves, as opposed to the purpose it may serve in the future.

ok


- Navis


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


On Jan. 22, 2014, 4:42 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 4:42 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 84ee78f 
>   conf/hive-default.xml.template 66d22f9 
>   service/src/java/org/apache/hive/service/AbstractService.java c2a2b2d 
>   service/src/java/org/apache/hive/service/cli/OperationState.java a023908 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 345617c 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java c8fb8ec 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 445c858 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java bfe0e7b 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Carl Steinbach <cw...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/#review32479
-----------------------------------------------------------



service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/15449/#comment61347>

    I think the name of the method should accurately reflect the purpose it currently serves, as opposed to the purpose it may serve in the future.


- Carl Steinbach


On Jan. 22, 2014, 4:42 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 4:42 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 84ee78f 
>   conf/hive-default.xml.template 66d22f9 
>   service/src/java/org/apache/hive/service/AbstractService.java c2a2b2d 
>   service/src/java/org/apache/hive/service/cli/OperationState.java a023908 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 345617c 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java c8fb8ec 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 445c858 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java bfe0e7b 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Aug. 27, 2014, 5:35 a.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 1584
> > <https://reviews.apache.org/r/15449/diff/8/?file=669977#file669977line1584>
> >
> >     Just curious:  for all 3 parameters, if the units are msec by default (as hive-default.xml.template said) then why are the default values shown as "0s" -- doesn't that mean 0 seconds, or is "s" something else?

Yes, seconds. I've revised code to accept time units like d/h/m/s/ms/us/ns and user should provide that. So there is no default timeunit, which is very confusional. I'm thinking of applying this policy to all other time configurations.


- Navis


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


On Aug. 27, 2014, 4:42 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2014, 4:42 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7f4afd9 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 45fbd61 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 21c33bc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 9785e95 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java eee1cc6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java bc0a02c 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java d573592 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

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



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

    Just curious:  for all 3 parameters, if the units are msec by default (as hive-default.xml.template said) then why are the default values shown as "0s" -- doesn't that mean 0 seconds, or is "s" something else?



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

    The parameter description needs the time unit information that used to be in hive-default.xml.template:  "Accepts a numeric value which is msec by default but also can be used with other time units appended (sec, min, hour, day)."



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

    Description needs unit information that was in hive-default.xml.template:  "Accepts a numeric value which is msec by default but also can be used with other time units appended (sec, min, hour, day)."



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

    Description needs unit information that was in hive-default.xml.template:  "Accepts a numeric value which is msec by default but also can be used with other time units appended (sec, min, hour, day)."


- Lefty Leverenz


On Aug. 27, 2014, 4:42 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2014, 4:42 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7f4afd9 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 45fbd61 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 21c33bc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 9785e95 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java eee1cc6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java bc0a02c 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java d573592 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Aug. 27, 2014, 2:32 p.m., Lars Francke wrote:
> > service/src/java/org/apache/hive/service/cli/session/SessionManager.java, line 112
> > <https://reviews.apache.org/r/15449/diff/9/?file=670083#file670083line112>
> >
> >     Hmm... this minimum should be documented in HiveConf and probably moved out to a constant. What's the reason for this minimum?

Someone had set 10 instead of 10000 and bad thing happened. I'll document this.


- Navis


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


On Aug. 27, 2014, 8:05 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2014, 8:05 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7f4afd9 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 45fbd61 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 21c33bc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 9785e95 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java eee1cc6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java bc0a02c 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java d573592 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Lars Francke <as...@lars-francke.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/#review51654
-----------------------------------------------------------



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

    Some of these lines are too long



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

    I suggest changing to "setting to zero or negative value."



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

    I suggest adding spaces between the elements of the enumeration to make it easier to read



common/src/java/org/apache/hadoop/hive/conf/Validator.java
<https://reviews.apache.org/r/15449/#comment90163>

    public static is redundant for interface members



itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java
<https://reviews.apache.org/r/15449/#comment90164>

    assertTrue can be replaced with fail() instead to make the error message clearer



itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java
<https://reviews.apache.org/r/15449/#comment90165>

    long line



service/src/java/org/apache/hive/service/cli/OperationState.java
<https://reviews.apache.org/r/15449/#comment90166>

    private is redundant for enum constructors



service/src/java/org/apache/hive/service/cli/operation/Operation.java
<https://reviews.apache.org/r/15449/#comment90167>

    no need to call the accessor, you can replace it just with lastAccessTime



service/src/java/org/apache/hive/service/cli/session/HiveSession.java
<https://reviews.apache.org/r/15449/#comment90168>

    public keyword is redundant in interfaces



service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java
<https://reviews.apache.org/r/15449/#comment90169>

    public keyword is redundant in interfaces



service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/15449/#comment90171>

    Missing @Override annotation



service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/15449/#comment90170>

    Assignments to longs are not atomic. You could ignore this, make lastAccessTime volatile or getLastAccessTime synchronized.
    
    http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.7



service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/15449/#comment90172>

    You can replace this with
    
    OperationHandle[] handles = opHandleSet.toArray(new OperationHandle[opHandleSet.size()]);
    
    to immediately create the proper sized array



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

    Hmm... this minimum should be documented in HiveConf and probably moved out to a constant. What's the reason for this minimum?



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

    Missing @Override


Mostly minor code style comments.

- Lars Francke


On Aug. 27, 2014, 8:05 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2014, 8:05 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7f4afd9 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 45fbd61 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 21c33bc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 9785e95 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java eee1cc6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java bc0a02c 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java d573592 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Lars Francke <as...@lars-francke.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/#review51882
-----------------------------------------------------------

Ship it!


Only minor comments mostly on lines exceeding Checkstyle's configuration.


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

    long line



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

    long line



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

    long line



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

    long lines



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

    long line



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

    long line



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

    long line



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

    long line



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

    long line



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

    long lines



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

    long line



common/src/java/org/apache/hadoop/hive/conf/Validator.java
<https://reviews.apache.org/r/15449/#comment90551>

    missing @Override



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/15449/#comment90552>

    long line



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/15449/#comment90553>

    long line



ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java
<https://reviews.apache.org/r/15449/#comment90554>

    This comment needs updating



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java
<https://reviews.apache.org/r/15449/#comment90555>

    extra space before semicolon



service/src/java/org/apache/hive/service/cli/operation/Operation.java
<https://reviews.apache.org/r/15449/#comment90556>

    "+ -" can be changed to just "-", maybe warrants a comment.


- Lars Francke


On Aug. 29, 2014, 9:05 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 9:05 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRetryingHMSHandler.java 39e7005 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 9e3481a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4e76236 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 84e6dcd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 063dee6 
>   metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 8287c60 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/AutoProgressor.java d7323cb 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java 7fdb4e7 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ScriptOperator.java 5b857e2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/UDTFOperator.java afd7bcf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 70047a2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java eb2851b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java ebe9f92 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 11434a0 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 46044d0 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java f636cff 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java db62721 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 3211759 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java f34b5ad 
>   ql/src/test/results/clientnegative/set_hiveconf_validation2.q.out 33f9360 
>   service/src/java/org/apache/hadoop/hive/service/HiveServer.java 32729f2 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 7668904 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 86ed4b4 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 21d1563 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Sept. 1, 2014, 5:52 a.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, lines 1496-1497
> > <https://reviews.apache.org/r/15449/diff/11-12/?file=672174#file672174line1496>
> >
> >     Mismatch between default units "1800s" and TimeValidator(TimeUnit.MILLISECONDS).

Argument for TimeValidator is default timeunit if user missed that (18000000 for example). It's ok.


- Navis


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


On Sept. 1, 2014, 5:14 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2014, 5:14 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestPermsGrp.java bf2b24e 
>   hcatalog/core/src/test/java/org/apache/hive/hcatalog/mapreduce/TestHCatPartitionPublish.java be7134f 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java a6a038a 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRetryingHMSHandler.java 39e7005 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 9ae6d7a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java a94a7a37 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java b9cf701 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 84e6dcd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java 5410b45 
>   metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 063dee6 
>   metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 8287c60 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/AutoProgressor.java d7323cb 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cd017d8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java 7fdb4e7 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ScriptOperator.java 5b857e2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/UDTFOperator.java afd7bcf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 70047a2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java eb2851b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java ebe9f92 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 11434a0 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 46044d0 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java f636cff 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java db62721 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 3211759 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java f34b5ad 
>   ql/src/test/results/clientpositive/show_conf.q.out a3c814a 
>   service/src/java/org/apache/hadoop/hive/service/HiveServer.java 32729f2 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 7668904 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java e5ce72f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 86ed4b4 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 21d1563 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

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



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

    Mismatch between default units "1800s" and TimeValidator(TimeUnit.MILLISECONDS).


- Lefty Leverenz


On Sept. 1, 2014, 5:14 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2014, 5:14 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestPermsGrp.java bf2b24e 
>   hcatalog/core/src/test/java/org/apache/hive/hcatalog/mapreduce/TestHCatPartitionPublish.java be7134f 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java a6a038a 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRetryingHMSHandler.java 39e7005 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 9ae6d7a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java a94a7a37 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java b9cf701 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 84e6dcd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java 5410b45 
>   metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 063dee6 
>   metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 8287c60 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/AutoProgressor.java d7323cb 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cd017d8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java 7fdb4e7 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ScriptOperator.java 5b857e2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/UDTFOperator.java afd7bcf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 70047a2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java eb2851b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java ebe9f92 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 11434a0 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 46044d0 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java f636cff 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java db62721 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 3211759 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java f34b5ad 
>   ql/src/test/results/clientpositive/show_conf.q.out a3c814a 
>   service/src/java/org/apache/hadoop/hive/service/HiveServer.java 32729f2 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 7668904 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java e5ce72f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 86ed4b4 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 21d1563 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

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

Ship it!


Ship It!

- Lefty Leverenz


On Sept. 1, 2014, 5:14 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2014, 5:14 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestPermsGrp.java bf2b24e 
>   hcatalog/core/src/test/java/org/apache/hive/hcatalog/mapreduce/TestHCatPartitionPublish.java be7134f 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java a6a038a 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRetryingHMSHandler.java 39e7005 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 9ae6d7a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java a94a7a37 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java b9cf701 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 84e6dcd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java 5410b45 
>   metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 063dee6 
>   metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 8287c60 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/AutoProgressor.java d7323cb 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cd017d8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java 7fdb4e7 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ScriptOperator.java 5b857e2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/UDTFOperator.java afd7bcf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 70047a2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java eb2851b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java ebe9f92 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 11434a0 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 46044d0 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java f636cff 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java db62721 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 3211759 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java f34b5ad 
>   ql/src/test/results/clientpositive/show_conf.q.out a3c814a 
>   service/src/java/org/apache/hadoop/hive/service/HiveServer.java 32729f2 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 7668904 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java e5ce72f 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 86ed4b4 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 21d1563 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Sept. 1, 2014, 5:14 a.m.)


Review request for hive.


Changes
-------

Fixed missing TimeValidators


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
  common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
  hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestPermsGrp.java bf2b24e 
  hcatalog/core/src/test/java/org/apache/hive/hcatalog/mapreduce/TestHCatPartitionPublish.java be7134f 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java a6a038a 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRetryingHMSHandler.java 39e7005 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 9ae6d7a 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java a94a7a37 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java b9cf701 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 84e6dcd 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java 5410b45 
  metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 063dee6 
  metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 8287c60 
  ql/src/java/org/apache/hadoop/hive/ql/exec/AutoProgressor.java d7323cb 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cd017d8 
  ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java 7fdb4e7 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ScriptOperator.java 5b857e2 
  ql/src/java/org/apache/hadoop/hive/ql/exec/UDTFOperator.java afd7bcf 
  ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 70047a2 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java eb2851b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java ebe9f92 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 11434a0 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 46044d0 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java f636cff 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java db62721 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 3211759 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java f34b5ad 
  ql/src/test/results/clientpositive/show_conf.q.out a3c814a 
  service/src/java/org/apache/hadoop/hive/service/HiveServer.java 32729f2 
  service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
  service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 7668904 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java e5ce72f 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 86ed4b4 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 21d1563 
  service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Aug. 31, 2014, 6:24 a.m., Lefty Leverenz wrote:
> >

All my bad. I hate meetings.


- Navis


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


On Aug. 29, 2014, 9:05 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 9:05 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRetryingHMSHandler.java 39e7005 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 9e3481a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4e76236 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 84e6dcd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 063dee6 
>   metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 8287c60 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/AutoProgressor.java d7323cb 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java 7fdb4e7 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ScriptOperator.java 5b857e2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/UDTFOperator.java afd7bcf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 70047a2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java eb2851b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java ebe9f92 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 11434a0 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 46044d0 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java f636cff 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java db62721 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 3211759 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java f34b5ad 
>   ql/src/test/results/clientnegative/set_hiveconf_validation2.q.out 33f9360 
>   service/src/java/org/apache/hadoop/hive/service/HiveServer.java 32729f2 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 7668904 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 86ed4b4 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 21d1563 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

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



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

    Shouldn't this have a TimeValidator?



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

    Again, no TimeValidator.



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

    No TimeValidator.



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

    No TimeValidator.



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

    Lack of TimeValidator here is deliberate, right?


- Lefty Leverenz


On Aug. 29, 2014, 9:05 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 9:05 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRetryingHMSHandler.java 39e7005 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 9e3481a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4e76236 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 84e6dcd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 063dee6 
>   metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 8287c60 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/AutoProgressor.java d7323cb 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java 7fdb4e7 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ScriptOperator.java 5b857e2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/UDTFOperator.java afd7bcf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 70047a2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java eb2851b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java ebe9f92 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 11434a0 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 46044d0 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java f636cff 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java db62721 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 3211759 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java f34b5ad 
>   ql/src/test/results/clientnegative/set_hiveconf_validation2.q.out 33f9360 
>   service/src/java/org/apache/hadoop/hive/service/HiveServer.java 32729f2 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 7668904 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 86ed4b4 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 21d1563 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Aug. 29, 2014, 9:05 a.m.)


Review request for hive.


Changes
-------

Addressed comments


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
  common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRetryingHMSHandler.java 39e7005 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 9e3481a 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4e76236 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 84e6dcd 
  metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 063dee6 
  metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 8287c60 
  ql/src/java/org/apache/hadoop/hive/ql/exec/AutoProgressor.java d7323cb 
  ql/src/java/org/apache/hadoop/hive/ql/exec/Heartbeater.java 7fdb4e7 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ScriptOperator.java 5b857e2 
  ql/src/java/org/apache/hadoop/hive/ql/exec/UDTFOperator.java afd7bcf 
  ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 70047a2 
  ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java eb2851b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java ebe9f92 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 11434a0 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java 46044d0 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java f636cff 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java db62721 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 3211759 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java f34b5ad 
  ql/src/test/results/clientnegative/set_hiveconf_validation2.q.out 33f9360 
  service/src/java/org/apache/hadoop/hive/service/HiveServer.java 32729f2 
  service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
  service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 7668904 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 86ed4b4 
  service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java 21d1563 
  service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> >
> 
> Navis Ryu wrote:
>     Addressing previous comments, I've revised validator to describe itself to description. For StringSet validator, the description of the conf will be started with something like, "Expects one of [textfile, sequencefile, rcfile, orc]." and for TimeValidator, it's "Expects a numeric value with timeunit (d/day, h/hour, m/min, s/sec, ms/msec, us/usec, ns/nsec)", etc. It's the reason why some part of description is removed. Could you generate the template and see the result? (cd commmon;mvn clean package -Phadoop-2 -Pdist -DskipTests). If you don't like this, I'll revert that.
> 
> Lefty Leverenz wrote:
>     Navis, that is cool to the nth degree!  I applied patch 15, generated a template file, and checked each parameter changed by the patch.  All the "Expects" phrases look great.
>     
>     However, non-numeric values are lowercase.  For example, hive.exec.orc.encoding.strategy used to say the values are SPEED and COMPRESSION, but now it's "Expects one of [speed, compression]."  Are all parameter values case-insensitive?  If so, the Configuration Properties & Configuration docs should mention it.
>     
>     Two parameters still give units in their descriptions, although that seems to be deliberate:
>     
>       - hive.server2.long.polling.timeout:  "Time in milliseconds that HiveServer2 will wait, ..." (has a non-zero default value, in milliseconds)
>       - hive.support.quoted.identifiers:  "Whether to use quoted identifier. 'none' or 'column' can be used." (goes on to explain what 'none' and 'column' mean)

bq. non-numeric values are lowercase
All values in StringSet are case-insensitive but if you prefer uppercase strings in description, that will be done. 

bq. Two parameters still give units in their descriptions
I was thinking of following issue to applying time validators to others, but I'll do that in here.


- Navis


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


On Aug. 28, 2014, 2:31 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2014, 2:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> >

Addressing previous comments, I've revised validator to describe itself to description. For StringSet validator, the description of the conf will be started with something like, "Expects one of [textfile, sequencefile, rcfile, orc]." and for TimeValidator, it's "Expects a numeric value with timeunit (d/day, h/hour, m/min, s/sec, ms/msec, us/usec, ns/nsec)", etc. It's the reason why some part of description is removed. Could you generate the template and see the result? (cd commmon;mvn clean package -Phadoop-2 -Pdist -DskipTests). If you don't like this, I'll revert that.


- Navis


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


On Aug. 28, 2014, 2:31 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2014, 2:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Lefty Leverenz <le...@gmail.com>.

> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> >
> 
> Navis Ryu wrote:
>     Addressing previous comments, I've revised validator to describe itself to description. For StringSet validator, the description of the conf will be started with something like, "Expects one of [textfile, sequencefile, rcfile, orc]." and for TimeValidator, it's "Expects a numeric value with timeunit (d/day, h/hour, m/min, s/sec, ms/msec, us/usec, ns/nsec)", etc. It's the reason why some part of description is removed. Could you generate the template and see the result? (cd commmon;mvn clean package -Phadoop-2 -Pdist -DskipTests). If you don't like this, I'll revert that.
> 
> Lefty Leverenz wrote:
>     Navis, that is cool to the nth degree!  I applied patch 15, generated a template file, and checked each parameter changed by the patch.  All the "Expects" phrases look great.
>     
>     However, non-numeric values are lowercase.  For example, hive.exec.orc.encoding.strategy used to say the values are SPEED and COMPRESSION, but now it's "Expects one of [speed, compression]."  Are all parameter values case-insensitive?  If so, the Configuration Properties & Configuration docs should mention it.
>     
>     Two parameters still give units in their descriptions, although that seems to be deliberate:
>     
>       - hive.server2.long.polling.timeout:  "Time in milliseconds that HiveServer2 will wait, ..." (has a non-zero default value, in milliseconds)
>       - hive.support.quoted.identifiers:  "Whether to use quoted identifier. 'none' or 'column' can be used." (goes on to explain what 'none' and 'column' mean)
> 
> Navis Ryu wrote:
>     bq. non-numeric values are lowercase
>     All values in StringSet are case-insensitive but if you prefer uppercase strings in description, that will be done. 
>     
>     bq. Two parameters still give units in their descriptions
>     I was thinking of following issue to applying time validators to others, but I'll do that in here.

bq. ... if you prefer uppercase strings in description, that will be done.

No, lowercase is better.  (If values appeared in uppercase, people would assume it's required.)


- Lefty


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


On Aug. 28, 2014, 2:31 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2014, 2:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Lefty Leverenz <le...@gmail.com>.

> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 1523
> > <https://reviews.apache.org/r/15449/diff/10/?file=670860#file670860line1523>
> >
> >     Please restore "(in seconds)" to description and specify other time units that can be used, if any.

Not an issue -- my mistake.


> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 1529
> > <https://reviews.apache.org/r/15449/diff/10/?file=670860#file670860line1529>
> >
> >     Please restore "(in seconds)" to description and specify other time units that can be used, if any.

Not an issue -- my mistake.


> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 1601
> > <https://reviews.apache.org/r/15449/diff/10/?file=670860#file670860line1601>
> >
> >     Please add time unit information:  "Accepts time units like d/h/m/s/ms/us/ns."

Not an issue -- my mistake.


> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 1604
> > <https://reviews.apache.org/r/15449/diff/10/?file=670860#file670860line1604>
> >
> >     Please add time unit information:  "Accepts time units like d/h/m/s/ms/us/ns."

Not an issue -- my mistake.


> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 1607
> > <https://reviews.apache.org/r/15449/diff/10/?file=670860#file670860line1607>
> >
> >     Please add time unit information:  "Accepts time units like d/h/m/s/ms/us/ns."

Not an issue -- my mistake.


- Lefty


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


On Aug. 28, 2014, 2:31 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2014, 2:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Lefty Leverenz <le...@gmail.com>.

> On Aug. 28, 2014, 7:56 a.m., Lefty Leverenz wrote:
> >
> 
> Navis Ryu wrote:
>     Addressing previous comments, I've revised validator to describe itself to description. For StringSet validator, the description of the conf will be started with something like, "Expects one of [textfile, sequencefile, rcfile, orc]." and for TimeValidator, it's "Expects a numeric value with timeunit (d/day, h/hour, m/min, s/sec, ms/msec, us/usec, ns/nsec)", etc. It's the reason why some part of description is removed. Could you generate the template and see the result? (cd commmon;mvn clean package -Phadoop-2 -Pdist -DskipTests). If you don't like this, I'll revert that.

Navis, that is cool to the nth degree!  I applied patch 15, generated a template file, and checked each parameter changed by the patch.  All the "Expects" phrases look great.

However, non-numeric values are lowercase.  For example, hive.exec.orc.encoding.strategy used to say the values are SPEED and COMPRESSION, but now it's "Expects one of [speed, compression]."  Are all parameter values case-insensitive?  If so, the Configuration Properties & Configuration docs should mention it.

Two parameters still give units in their descriptions, although that seems to be deliberate:

  - hive.server2.long.polling.timeout:  "Time in milliseconds that HiveServer2 will wait, ..." (has a non-zero default value, in milliseconds)
  - hive.support.quoted.identifiers:  "Whether to use quoted identifier. 'none' or 'column' can be used." (goes on to explain what 'none' and 'column' mean)


- Lefty


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


On Aug. 28, 2014, 2:31 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2014, 2:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

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



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

    Why did you remove the possible options?



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

    Why did you remove the possible values?



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

    Why did you remove the possible options?



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

    Nit:  use camel caps on HiveServer2.



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

    Please restore "(in seconds)" to description and specify other time units that can be used, if any.



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

    Please restore "(in seconds)" to description and specify other time units that can be used, if any.



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

    Please add time unit information:  "Accepts time units like d/h/m/s/ms/us/ns."



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

    Please add time unit information:  "Accepts time units like d/h/m/s/ms/us/ns."



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

    Please add time unit information:  "Accepts time units like d/h/m/s/ms/us/ns."


- Lefty Leverenz


On Aug. 28, 2014, 2:31 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2014, 2:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
>   common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Aug. 28, 2014, 2:31 a.m.)


Review request for hive.


Changes
-------

Addressed comments


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/ant/GenHiveTemplate.java 4293b7c 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 74bb863 
  common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/CLIService.java ff5de4a 
  service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 0d6436e 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 2867301 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 270e4a6 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 84e1c7e 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 4e5f595 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 17c1c7b 
  service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d01e819 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Aug. 27, 2014, 8:05 a.m.)


Review request for hive.


Changes
-------

Addressed comments


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7f4afd9 
  common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 45fbd61 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 21c33bc 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 9785e95 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java eee1cc6 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java bc0a02c 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java d573592 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Aug. 27, 2014, 4:42 a.m.)


Review request for hive.


Changes
-------

Addressed comments


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7f4afd9 
  common/src/java/org/apache/hadoop/hive/conf/Validator.java cea9c41 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/miniHS2/TestHiveServer2SessionTimeout.java PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 45fbd61 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 21c33bc 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 9785e95 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java eee1cc6 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java bc0a02c 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java 39d2184 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java d573592 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

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



conf/hive-default.xml.template
<https://reviews.apache.org/r/15449/#comment89785>

    hive-default.xml.template should not be included in this patch -- instead, the parameter descriptions should be included with the parameters in HiveConf.java (ever since HIVE-6037, which you can check for multi-line description syntax)


- Lefty Leverenz


On April 3, 2014, 5:35 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated April 3, 2014, 5:35 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 33d7e5f 
>   conf/hive-default.xml.template 05c154e 
>   service/src/java/org/apache/hive/service/AbstractService.java c2a2b2d 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java d6651ba 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 21c33bc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 9785e95 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java a5c8e9b 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java f730119 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 7f6687e 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated April 3, 2014, 5:35 a.m.)


Review request for hive.


Changes
-------

Rebased to trunk & fixed typos in template.


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 33d7e5f 
  conf/hive-default.xml.template 05c154e 
  service/src/java/org/apache/hive/service/AbstractService.java c2a2b2d 
  service/src/java/org/apache/hive/service/cli/OperationState.java 3e15f0c 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java d6651ba 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 21c33bc 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 9785e95 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java a5c8e9b 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java f730119 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 7f6687e 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Jan. 24, 2014, 12:44 a.m.)


Review request for hive.


Changes
-------

1. Fixed typos in templates
2. Fixed bug in checking operation timeout when timeout value is minus
3. Set thread name for session checker


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 371cb0f 
  conf/hive-default.xml.template 66d22f9 
  service/src/java/org/apache/hive/service/AbstractService.java c2a2b2d 
  service/src/java/org/apache/hive/service/cli/OperationState.java a023908 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 345617c 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java c8fb8ec 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 445c858 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java bfe0e7b 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Jan. 22, 2014, 7:23 a.m.)


Review request for hive.


Changes
-------

Addressed comment


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 84ee78f 
  conf/hive-default.xml.template 66d22f9 
  service/src/java/org/apache/hive/service/AbstractService.java c2a2b2d 
  service/src/java/org/apache/hive/service/cli/OperationState.java a023908 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 345617c 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java c8fb8ec 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 445c858 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java bfe0e7b 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Jan. 22, 2014, 4:42 a.m.)


Review request for hive.


Changes
-------

Addressed some comments


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 84ee78f 
  conf/hive-default.xml.template 66d22f9 
  service/src/java/org/apache/hive/service/AbstractService.java c2a2b2d 
  service/src/java/org/apache/hive/service/cli/OperationState.java a023908 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6 
  service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 345617c 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java c8fb8ec 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 445c858 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java bfe0e7b 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Nov. 18, 2013, 2:32 a.m.)


Review request for hive.


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
  conf/hive-default.xml.template c574ab5 
  service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 6f4b8dc 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 00058cc 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java cfda752 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java e262b72 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.

> On Nov. 15, 2013, 9:21 a.m., Carl Steinbach wrote:
> > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java, line 397
> > <https://reviews.apache.org/r/15449/diff/2/?file=382968#file382968line397>
> >
> >     Can we change the name to something like closeExpiredOperations()?

I think other things can be done here not just checking expiring.


> On Nov. 15, 2013, 9:21 a.m., Carl Steinbach wrote:
> > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java, line 405
> > <https://reviews.apache.org/r/15449/diff/2/?file=382968#file382968line405>
> >
> >     Is this actually necessary or is it a hypothetical bug you're guarding against? Is it possible to get rid of opHandleSet and push most of this logic down into OperationManager?

It's not introduced by this issue. But I moved logic itself to operation manager.


- Navis


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


On Nov. 18, 2013, 2:32 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2013, 2:32 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
>   conf/hive-default.xml.template c574ab5 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 6f4b8dc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 00058cc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java cfda752 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java e262b72 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Carl Steinbach <cw...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/#review28950
-----------------------------------------------------------



service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/15449/#comment56006>

    Can we change the name to something like closeExpiredOperations()?



service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/15449/#comment56007>

    Is this actually necessary or is it a hypothetical bug you're guarding against? Is it possible to get rid of opHandleSet and push most of this logic down into OperationManager?



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

    Change the name to timeoutChecker or sessionTimeoutChecker?


- Carl Steinbach


On Nov. 13, 2013, 1:21 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15449/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2013, 1:21 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5799
>     https://issues.apache.org/jira/browse/HIVE-5799
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Need some timeout facility for preventing resource leakages from instable or bad clients.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
>   conf/hive-default.xml.template c574ab5 
>   service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 6f4b8dc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 00058cc 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java cfda752 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 25c6f38 
> 
> Diff: https://reviews.apache.org/r/15449/diff/
> 
> 
> Testing
> -------
> 
> Confirmed in the local environment.
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15449: session/operation timeout for hiveserver2

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15449/
-----------------------------------------------------------

(Updated Nov. 13, 2013, 1:21 a.m.)


Review request for hive.


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


Repository: hive-git


Description
-------

Need some timeout facility for preventing resource leakages from instable or bad clients.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
  conf/hive-default.xml.template c574ab5 
  service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 6f4b8dc 
  service/src/java/org/apache/hive/service/cli/session/HiveSession.java 00058cc 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java cfda752 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 25c6f38 

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


Testing
-------

Confirmed in the local environment.


Thanks,

Navis Ryu