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