----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15151/#review28168 -----------------------------------------------------------
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15151/#comment54799> This and some other variables used by async execution needs to be made volatile, as it can now be accessed from multiple threads. We should also do that for state in Operation.java . But that can be addressed in another thread. service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15151/#comment54798> we should make this volatile. This can be accessed from multiple threads. service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceClient.java <https://reviews.apache.org/r/15151/#comment54800> why is it returning null instead of the exception ? service/src/test/org/apache/hive/service/cli/CLIServiceTest.java <https://reviews.apache.org/r/15151/#comment54802> assertEquals("operation state", OperationState.ERROR, state) will give a more informative error if test fails. service/src/test/org/apache/hive/service/cli/CLIServiceTest.java <https://reviews.apache.org/r/15151/#comment54801> can you also add a check for the error message ? service/src/test/org/apache/hive/service/cli/thrift/ThriftCLIServiceTest.java <https://reviews.apache.org/r/15151/#comment54803> can you add a check on the error message string here ? - Thejas Nair On Nov. 1, 2013, 12:54 a.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15151/ > ----------------------------------------------------------- > > (Updated Nov. 1, 2013, 12:54 a.m.) > > > Review request for hive, Prasad Mujumdar and Thejas Nair. > > > Bugs: HIVE-5230 > https://issues.apache.org/jira/browse/HIVE-5230 > > > Repository: hive-git > > > Description > ------- > > [HIVE-4617|https://issues.apache.org/jira/browse/HIVE-4617] provides support > for async execution in HS2. When a background thread gets an error, currently > the client can only poll for the operation state and also the error with its > stacktrace is logged. However, it will be useful to provide a richer error > response like thrift API does with TStatus (which is constructed while > building a Thrift response object). > > > Diffs > ----- > > service/src/java/org/apache/hive/service/cli/CLIService.java 1a7f338 > service/src/java/org/apache/hive/service/cli/CLIServiceClient.java 14ef54f > service/src/java/org/apache/hive/service/cli/EmbeddedCLIServiceClient.java > 9dca874 > service/src/java/org/apache/hive/service/cli/ICLIService.java f647ce6 > service/src/java/org/apache/hive/service/cli/OperationStatus.java > PRE-CREATION > service/src/java/org/apache/hive/service/cli/operation/Operation.java > 6f4b8dc > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > bcdb67f > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > f6adf92 > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java > 9df110e > > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceClient.java > 9bb2a0f > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java d6caed1 > > service/src/test/org/apache/hive/service/cli/thrift/ThriftCLIServiceTest.java > ff7166d > > Diff: https://reviews.apache.org/r/15151/diff/ > > > Testing > ------- > > > Thanks, > > Vaibhav Gumashta > >
