----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19395/#review37713 -----------------------------------------------------------
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. service/src/java/org/apache/hive/service/cli/operation/OperationManager.java <https://reviews.apache.org/r/19395/#comment69359> Should the log message include operation handle as well ? Just the state won't be very helpful for analyzing logs .. service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/19395/#comment69360> 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 ? - Prasad Mujumdar 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 > >
