hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rajat Khandelwal <rajatgupt...@gmail.com>
Subject Re: Review Request 45738: HIVE-13415: Decouple Sessions from thrift binary transport
Date Wed, 06 Apr 2016 07:36:47 GMT

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

(Updated April 6, 2016, 1:06 p.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 (updated)
-----

  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