hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Amareshwari Sriramadasu <amareshw...@apache.org>
Subject Re: Review Request 45738: HIVE-13415: Decouple Sessions from thrift binary transport
Date Mon, 11 Apr 2016 06:09:37 GMT

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




service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
<https://reviews.apache.org/r/45738/#comment191464>

    Why is this code movement to other class?



service/src/test/org/apache/hive/service/cli/TestRetryingThriftCLIServiceClient.java (line
174)
<https://reviews.apache.org/r/45738/#comment191463>

    If client transport is closed, how are you able to do closeSession? Shouldnt you create
a new client?


- Amareshwari Sriramadasu


On April 6, 2016, 7:36 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45738/
> -----------------------------------------------------------
> 
> (Updated April 6, 2016, 7:36 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-13415
>     https://issues.apache.org/jira/browse/HIVE-13415
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Current behaviour is:
> 
> * Open a thrift binary transport
> * create a session
> * close the transport
> 
> Then the session gets closed. Consequently, all the operations running in the session
also get killed.
> 
> Whereas, if you open an HTTP transport, and close, the enclosing sessions are not closed.

> 
> This seems like a bad design, having transport and sessions tightly coupled. I'd like
to fix this. 
> 
> The issue that introduced it is [HIVE-9601|https://github.com/apache/hive/commit/48bea00c48853459af64b4ca9bfdc3e821c4ed82]
Relevant discussions at [here|https://issues.apache.org/jira/browse/HIVE-11485?focusedCommentId=15223546&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15223546],
[here|https://issues.apache.org/jira/browse/HIVE-11485?focusedCommentId=15223827&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15223827]
and mentioned links on those comments. 
> 
> Another thing that seems like a slightly bad design is this line of code in ThriftBinaryCLIService:
> 
> {noformat}
> server.setServerEventHandler(serverEventHandler);
> {noformat}
> 
> Whereas serverEventHandler is defined by the base class, with no users except one sub-class(ThriftBinaryCLIService),
violating the separation of concerns.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 95c5c0efddf9cb91786d5379ecf7c5af50a315ea

>   service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java cf575a405f3eed2e1ec470670d5832ec3af9e1c1

>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 0a2a761867ab596e942dbc78eaabc0ef920665a3

>   service/src/test/org/apache/hive/service/cli/TestRetryingThriftCLIServiceClient.java
3bd82e614a50c0b5419a926ff00a95dafd4b0ebb 
> 
> Diff: https://reviews.apache.org/r/45738/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


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