----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15337/#review28505 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/15337/#comment55325> * 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? service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15337/#comment55326> Formatting (here and in each of the following catch clauses). service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15337/#comment55327> Including the operation handle Id in each of these logging messages would be nice. See CLIService for an example of that. service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15337/#comment55329> 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. service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15337/#comment55330> Same question as before. service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15337/#comment55331> I'm not sure this is worth logging. Please consider either removing or changing to trace level. service/src/test/org/apache/hive/service/cli/CLIServiceTest.java <https://reviews.apache.org/r/15337/#comment55332> 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. service/src/test/org/apache/hive/service/cli/CLIServiceTest.java <https://reviews.apache.org/r/15337/#comment55328> Please reference HIVE_SERVER2_LONG_POLLING_TIMEOUT instead. - Carl Steinbach 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 > >
