hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carl Steinbach" <c...@cloudera.com>
Subject Re: Review Request 15337: HIVE-5217: Add long polling to asynchronous execution in HiveServer2
Date Fri, 08 Nov 2013 06:37:59 GMT

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



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

    * It looks like SQLOperation.getState() is the *only* call that supports this behavior,
but the description implies that this applies to multiple calls.
    
    * "SQLOperation#getState" is an internal name that probably won't make much sense to the
majority of people reading this.
    
    * It can't hurt to say that this configuration only applies if the statement is being
executed in asynchronous mode.
    
    * It looks like this enables long polling by default for all sessions. Is that the intent?
    
    



service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
<https://reviews.apache.org/r/15337/#comment55326>

    Formatting (here and in each of the following catch clauses).



service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
<https://reviews.apache.org/r/15337/#comment55327>

    Including the operation handle Id in each of these logging messages would be nice. See
CLIService for an example of that.



service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
<https://reviews.apache.org/r/15337/#comment55329>

    Does this mean that the client canceled the operation? If so it seems likely that this
was already logged somewhere else. If not, then this should probably be logged at the trace
level.



service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
<https://reviews.apache.org/r/15337/#comment55330>

    Same question as before.



service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
<https://reviews.apache.org/r/15337/#comment55331>

    I'm not sure this is worth logging. Please consider either removing or changing to trace
level.



service/src/test/org/apache/hive/service/cli/CLIServiceTest.java
<https://reviews.apache.org/r/15337/#comment55332>

    I think this needs to be split into three separate cases: 1) the default behavior (whatever
that is), 2) hive.server2.long.polling.timeout = 0, and hive.server2.long.polling.timeout
> 0.
    
    



service/src/test/org/apache/hive/service/cli/CLIServiceTest.java
<https://reviews.apache.org/r/15337/#comment55328>

    Please reference HIVE_SERVER2_LONG_POLLING_TIMEOUT instead.


- Carl Steinbach


On Nov. 8, 2013, 5:50 a.m., Carl Steinbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15337/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2013, 5:50 a.m.)
> 
> 
> Review request for hive and Vaibhav Gumashta.
> 
> 
> Bugs: HIVE-5217
>     https://issues.apache.org/jira/browse/HIVE-5217
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add long polling to asynchronous execution in HiveServer2
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b1fd468 
>   conf/hive-default.xml.template fe7141e 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 4ee1b74 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java cd9d99a 
> 
> Diff: https://reviews.apache.org/r/15337/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Carl Steinbach
> 
>


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