----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45738/#review128632 -----------------------------------------------------------
Ship it! Changes look good to me. Carl, Szehon, can you also look at ? - Amareshwari Sriramadasu On April 13, 2016, 8:05 a.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45738/ > ----------------------------------------------------------- > > (Updated April 13, 2016, 8:05 a.m.) > > > Review request for hive, Carl Steinbach and Szehon Ho. > > > 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 > c7e5b3305f2443a63b1f3210b3f2fdd563cab965 > > service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java > cf575a405f3eed2e1ec470670d5832ec3af9e1c1 > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java > be9833d1ac28951cb464716ddab4a7ea9d6b0a93 > > service/src/test/org/apache/hive/service/cli/TestRetryingThriftCLIServiceClient.java > 3bd82e614a50c0b5419a926ff00a95dafd4b0ebb > > Diff: https://reviews.apache.org/r/45738/diff/ > > > Testing > ------- > > > Thanks, > > Rajat Khandelwal > >
