> On Nov. 8, 2013, 6:37 a.m., Carl Steinbach wrote: > > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java, > > line 224 > > <https://reviews.apache.org/r/15337/diff/1/?file=380781#file380781line224> > > > > Formatting (here and in each of the following catch clauses).
Done > On Nov. 8, 2013, 6:37 a.m., Carl Steinbach wrote: > > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java, > > line 226 > > <https://reviews.apache.org/r/15337/diff/1/?file=380781#file380781line226> > > > > Including the operation handle Id in each of these logging messages > > would be nice. See CLIService for an example of that. Done > On Nov. 8, 2013, 6:37 a.m., Carl Steinbach wrote: > > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java, > > line 230 > > <https://reviews.apache.org/r/15337/diff/1/?file=380781#file380781line230> > > > > Does this mean that the client canceled the operation? If so it seems > > likely that this was already logged somewhere else. If not, then this > > should probably be logged at the trace level. Done > On Nov. 8, 2013, 6:37 a.m., Carl Steinbach wrote: > > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java, > > line 234 > > <https://reviews.apache.org/r/15337/diff/1/?file=380781#file380781line234> > > > > Same question as before. Done > On Nov. 8, 2013, 6:37 a.m., Carl Steinbach wrote: > > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java, > > line 239 > > <https://reviews.apache.org/r/15337/diff/1/?file=380781#file380781line239> > > > > I'm not sure this is worth logging. Please consider either removing or > > changing to trace level. Done > On Nov. 8, 2013, 6:37 a.m., Carl Steinbach wrote: > > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 163 > > <https://reviews.apache.org/r/15337/diff/1/?file=380782#file380782line163> > > > > Please reference HIVE_SERVER2_LONG_POLLING_TIMEOUT instead. We're setting it to a lower value since the test query is a short one and we would like to get a handle on the operation moving through different states. > On Nov. 8, 2013, 6:37 a.m., Carl Steinbach wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 768 > > <https://reviews.apache.org/r/15337/diff/1/?file=380779#file380779line768> > > > > * It looks like SQLOperation.getState() is the *only* call that > > supports this behavior, but the description implies that this applies to > > multiple calls. > > > > * "SQLOperation#getState" is an internal name that probably won't make > > much sense to the majority of people reading this. > > > > * It can't hurt to say that this configuration only applies if the > > statement is being executed in asynchronous mode. > > > > * It looks like this enables long polling by default for all sessions. > > Is that the intent? > > > > Done. Yes the intent is to rely on a configurable value for all the sessions. > On Nov. 8, 2013, 6:37 a.m., Carl Steinbach wrote: > > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 160 > > <https://reviews.apache.org/r/15337/diff/1/?file=380782#file380782line160> > > > > I think this needs to be split into three separate cases: 1) the > > default behavior (whatever that is), 2) hive.server2.long.polling.timeout = > > 0, and hive.server2.long.polling.timeout > 0. > > > > Done - Vaibhav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15337/#review28505 ----------------------------------------------------------- On Nov. 8, 2013, 5:50 a.m., Carl Steinbach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15337/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2013, 5:50 a.m.) > > > Review request for hive and Vaibhav Gumashta. > > > Bugs: HIVE-5217 > https://issues.apache.org/jira/browse/HIVE-5217 > > > Repository: hive-git > > > Description > ------- > > Add long polling to asynchronous execution in HiveServer2 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b1fd468 > conf/hive-default.xml.template fe7141e > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > 4ee1b74 > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java cd9d99a > > Diff: https://reviews.apache.org/r/15337/diff/ > > > Testing > ------- > > > Thanks, > > Carl Steinbach > >
