> On March 19, 2014, 3:01 p.m., Prasad Mujumdar wrote: > > Looks fine overall. A couple of minor comments below. > > Do we have a test for asynchronous cancel() ? You might want to log a > > followup ticket to add test cases.
I think it makes sense to add a test in this ticket itself. I did test it with a standalone jdbc client program, let me think of a good way to unit test it (maybe a sleep udf?). > On March 19, 2014, 3:01 p.m., Prasad Mujumdar wrote: > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java, > > line 163 > > <https://reviews.apache.org/r/19395/diff/1/?file=527980#file527980line163> > > > > Should the log message include operation handle as well ? Just the > > state won't be very helpful for analyzing logs .. Sure, will do. > On March 19, 2014, 3:01 p.m., Prasad Mujumdar wrote: > > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java, > > line 150 > > <https://reviews.apache.org/r/19395/diff/1/?file=527981#file527981line150> > > > > As part of this patch, the client is already serializing cancel() as > > well as the cancelOperation() is validating the state before executing the > > cancel. > > In that case, do we need this check again ? The serialization within cancel (& elsewhere) was required since the same client socket is used for all the rpc calls during the session. However, this check is more for the thread running the execute call. In the case when a running operation is canceled by the other thread, we don't want the running thread to throw an error. - Vaibhav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19395/#review37713 ----------------------------------------------------------- On March 19, 2014, 11:02 a.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19395/ > ----------------------------------------------------------- > > (Updated March 19, 2014, 11:02 a.m.) > > > Review request for hive, Prasad Mujumdar and Thejas Nair. > > > Bugs: HIVE-6472 > https://issues.apache.org/jira/browse/HIVE-6472 > > > Repository: hive-git > > > Description > ------- > > https://issues.apache.org/jira/browse/HIVE-6472 > > > Diffs > ----- > > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 01e6ea7 > service/src/java/org/apache/hive/service/cli/OperationState.java a023908 > > service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java > 89f2ae9 > service/src/java/org/apache/hive/service/cli/operation/Operation.java > 3f36e2d > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > 345617c > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > ace791a > > Diff: https://reviews.apache.org/r/19395/diff/ > > > Testing > ------- > > > Thanks, > > Vaibhav Gumashta > >
