You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vaibhav Gumashta <vg...@hortonworks.com> on 2013/11/12 00:58:19 UTC
Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/
-----------------------------------------------------------
Review request for hive, Carl Steinbach and Thejas Nair.
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 4f32390
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/15435/diff/
Testing
-------
Thanks,
Vaibhav Gumashta
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/#review34933
-----------------------------------------------------------
service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java
<https://reviews.apache.org/r/15435/#comment65313>
it would be better to call the constructor with more arguments from this one.
service/src/java/org/apache/hive/service/cli/operation/Operation.java
<https://reviews.apache.org/r/15435/#comment65311>
I think we should make runAsync field final, as it is not supposed to change.
service/src/java/org/apache/hive/service/cli/operation/Operation.java
<https://reviews.apache.org/r/15435/#comment65312>
it think it is cleaner to have this constructor call (instead of the other way around)
this(parentSession, opType, false);
That way initialization will be in one constructor, and it will be clear what all variables get initalized to.
- Thejas Nair
On Feb. 18, 2014, 1:16 p.m., Vaibhav Gumashta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15435/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2014, 1:16 p.m.)
>
>
> Review request for hive, Carl Steinbach and Thejas Nair.
>
>
> 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 a182cd7
> service/src/java/org/apache/hive/service/cli/CLIService.java 56b357a
> service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java e973f83
> service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 03a37c8
> service/src/test/org/apache/hive/service/cli/CLIServiceTest.java 8ec8d43
>
> Diff: https://reviews.apache.org/r/15435/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vaibhav Gumashta
>
>
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/#review35050
-----------------------------------------------------------
Ship it!
Ship It!
- Thejas Nair
On Feb. 20, 2014, 2:49 a.m., Vaibhav Gumashta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15435/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 2:49 a.m.)
>
>
> Review request for hive, Carl Steinbach and Thejas Nair.
>
>
> 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 a182cd7
> conf/hive-default.xml.template 0d08aa2
> service/src/java/org/apache/hive/service/cli/CLIService.java 56b357a
> service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java e973f83
> service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java c6e1692
> service/src/java/org/apache/hive/service/cli/operation/MetadataOperation.java 8dc82ab
> service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6
> service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 03a37c8
> service/src/test/org/apache/hive/service/cli/CLIServiceTest.java 8ec8d43
>
> Diff: https://reviews.apache.org/r/15435/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vaibhav Gumashta
>
>
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 2:49 a.m.)
Review request for hive, Carl Steinbach and Thejas Nair.
Changes
-------
Changes per feedback.
Bugs: HIVE-5217
https://issues.apache.org/jira/browse/HIVE-5217
Repository: hive-git
Description
-------
Add long polling to asynchronous execution in HiveServer2
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a182cd7
conf/hive-default.xml.template 0d08aa2
service/src/java/org/apache/hive/service/cli/CLIService.java 56b357a
service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java e973f83
service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java c6e1692
service/src/java/org/apache/hive/service/cli/operation/MetadataOperation.java 8dc82ab
service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 03a37c8
service/src/test/org/apache/hive/service/cli/CLIServiceTest.java 8ec8d43
Diff: https://reviews.apache.org/r/15435/diff/
Testing
-------
Thanks,
Vaibhav Gumashta
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/
-----------------------------------------------------------
(Updated Feb. 18, 2014, 1:16 p.m.)
Review request for hive, Carl Steinbach and Thejas Nair.
Changes
-------
Rebased on trunk + feedback changes
Bugs: HIVE-5217
https://issues.apache.org/jira/browse/HIVE-5217
Repository: hive-git
Description
-------
Add long polling to asynchronous execution in HiveServer2
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a182cd7
service/src/java/org/apache/hive/service/cli/CLIService.java 56b357a
service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java e973f83
service/src/java/org/apache/hive/service/cli/operation/Operation.java 58a28b6
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 03a37c8
service/src/test/org/apache/hive/service/cli/CLIServiceTest.java 8ec8d43
Diff: https://reviews.apache.org/r/15435/diff/
Testing
-------
Thanks,
Vaibhav Gumashta
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/
-----------------------------------------------------------
(Updated Nov. 14, 2013, 12:02 a.m.)
Review request for hive, Carl Steinbach and Thejas Nair.
Bugs: HIVE-5217
https://issues.apache.org/jira/browse/HIVE-5217
Repository: hive-git
Description
-------
Add long polling to asynchronous execution in HiveServer2
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 4f32390
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/15435/diff/
Testing
-------
Thanks,
Vaibhav Gumashta
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/
-----------------------------------------------------------
(Updated Nov. 13, 2013, 11:58 p.m.)
Review request for hive, Carl Steinbach and Thejas Nair.
Changes
-------
Changes based on feedback.
Bugs: HIVE-5217
https://issues.apache.org/jira/browse/HIVE-5217
Repository: hive-git
Description
-------
Add long polling to asynchronous execution in HiveServer2
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 4f32390
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/15435/diff/
Testing
-------
Thanks,
Vaibhav Gumashta
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 402
> > <https://reviews.apache.org/r/15435/diff/3/?file=382339#file382339line402>
> >
> > Please use a switch statement here.
We can use a switch here but in java enum constant cannot be qualified in a case label. Which means basically within each case we'll have to use case(CANCELED) instead of case(OperationState.CANCELED) [<-- not allowed]. I guess the current approach is better.
> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java, line 220
> > <https://reviews.apache.org/r/15435/diff/3/?file=382338#file382338line220>
> >
> > Should probably use a long and HiveConf.getLongVar instead of ints.
Done (although with int the max value could reach in several hours. Not sure if we are looking for a timeout of more than that).
> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 356
> > <https://reviews.apache.org/r/15435/diff/3/?file=382339#file382339line356>
> >
> > Please try to avoid using concrete collection types on the LHS or assignments or in method parameter lists, i.e. use Map<String, String> instead of HashMap<String, String>.
Done
> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 384
> > <https://reviews.apache.org/r/15435/diff/3/?file=382339#file382339line384>
> >
> > Replace "hive.server2.long.polling.timeout" with HiveConf.ConfVars.HIVE_SERVER2_LONG_POLLING_TIMEOUT.varname.
Done
> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 406
> > <https://reviews.apache.org/r/15435/diff/3/?file=382339#file382339line406>
> >
> > Formatting.
Done
- Vaibhav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/#review28749
-----------------------------------------------------------
On Nov. 12, 2013, 2:16 a.m., Vaibhav Gumashta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15435/
> -----------------------------------------------------------
>
> (Updated Nov. 12, 2013, 2:16 a.m.)
>
>
> Review request for hive, Carl Steinbach and Thejas Nair.
>
>
> 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 4f32390
> 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/15435/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vaibhav Gumashta
>
>
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Carl Steinbach <cw...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/#review28749
-----------------------------------------------------------
service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
<https://reviews.apache.org/r/15435/#comment55768>
Should probably use a long and HiveConf.getLongVar instead of ints.
service/src/test/org/apache/hive/service/cli/CLIServiceTest.java
<https://reviews.apache.org/r/15435/#comment55770>
Please try to avoid using concrete collection types on the LHS or assignments or in method parameter lists, i.e. use Map<String, String> instead of HashMap<String, String>.
service/src/test/org/apache/hive/service/cli/CLIServiceTest.java
<https://reviews.apache.org/r/15435/#comment55742>
Please remove TABs
service/src/test/org/apache/hive/service/cli/CLIServiceTest.java
<https://reviews.apache.org/r/15435/#comment55767>
Replace "hive.server2.long.polling.timeout" with HiveConf.ConfVars.HIVE_SERVER2_LONG_POLLING_TIMEOUT.varname.
service/src/test/org/apache/hive/service/cli/CLIServiceTest.java
<https://reviews.apache.org/r/15435/#comment55764>
Please use a switch statement here.
service/src/test/org/apache/hive/service/cli/CLIServiceTest.java
<https://reviews.apache.org/r/15435/#comment55769>
Formatting.
- Carl Steinbach
On Nov. 12, 2013, 2:16 a.m., Vaibhav Gumashta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15435/
> -----------------------------------------------------------
>
> (Updated Nov. 12, 2013, 2:16 a.m.)
>
>
> Review request for hive, Carl Steinbach and Thejas Nair.
>
>
> 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 4f32390
> 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/15435/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vaibhav Gumashta
>
>
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/
-----------------------------------------------------------
(Updated Nov. 12, 2013, 2:16 a.m.)
Review request for hive, Carl Steinbach and Thejas Nair.
Changes
-------
Refactoring of tests.
Bugs: HIVE-5217
https://issues.apache.org/jira/browse/HIVE-5217
Repository: hive-git
Description
-------
Add long polling to asynchronous execution in HiveServer2
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 4f32390
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/15435/diff/
Testing
-------
Thanks,
Vaibhav Gumashta
Re: Review Request 15435: Add long polling to asynchronous execution in
HiveServer2
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/
-----------------------------------------------------------
(Updated Nov. 12, 2013, 12:06 a.m.)
Review request for hive, Carl Steinbach and Thejas Nair.
Changes
-------
Lint issues.
Bugs: HIVE-5217
https://issues.apache.org/jira/browse/HIVE-5217
Repository: hive-git
Description
-------
Add long polling to asynchronous execution in HiveServer2
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 4f32390
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/15435/diff/
Testing
-------
Thanks,
Vaibhav Gumashta