You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Siying Dong <si...@fb.com> on 2011/04/22 02:11:43 UTC
Review Request: CommandNeedRetryException needs release locks and some
related code cleaning up
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/646/
-----------------------------------------------------------
Review request for hive, Yongqiang He and namit jain.
Summary
-------
now when CommandNeedRetryException is thrown, locks are not released. Not sure whether it will cause problem, since the same locks will be acquired when retrying it. It is anyway something we need to fix. Also we can do some little code cleaning up to make future mistakes less likely.
This addresses bug HIVE-2123.
https://issues.apache.org/jira/browse/HIVE-2123
Diffs
-----
trunk/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 1095838
trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1095838
Diff: https://reviews.apache.org/r/646/diff
Testing
-------
Thanks,
Siying
Re: Review Request: CommandNeedRetryException needs release locks and some
related code cleaning up
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/646/#review527
-----------------------------------------------------------
trunk/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
<https://reviews.apache.org/r/646/#comment1091>
There's a try/catch block here, so why not do this the Java way and throw an exception instead of returning a status code?
trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java
<https://reviews.apache.org/r/646/#comment1092>
Can you replace this with a DriverException class?
trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java
<https://reviews.apache.org/r/646/#comment1093>
spacing
- Carl
On 2011-04-22 00:11:43, Siying Dong wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/646/
> -----------------------------------------------------------
>
> (Updated 2011-04-22 00:11:43)
>
>
> Review request for hive, Yongqiang He and namit jain.
>
>
> Summary
> -------
>
> now when CommandNeedRetryException is thrown, locks are not released. Not sure whether it will cause problem, since the same locks will be acquired when retrying it. It is anyway something we need to fix. Also we can do some little code cleaning up to make future mistakes less likely.
>
>
> This addresses bug HIVE-2123.
> https://issues.apache.org/jira/browse/HIVE-2123
>
>
> Diffs
> -----
>
> trunk/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 1095838
> trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1095838
>
> Diff: https://reviews.apache.org/r/646/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Siying
>
>
Re: Review Request: CommandNeedRetryException needs release locks and some
related code cleaning up
Posted by Siying Dong <si...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/646/
-----------------------------------------------------------
(Updated 2011-04-22 04:22:16.136899)
Review request for hive, Yongqiang He and namit jain.
Changes
-------
execute() not to throw any exception.
Summary
-------
now when CommandNeedRetryException is thrown, locks are not released. Not sure whether it will cause problem, since the same locks will be acquired when retrying it. It is anyway something we need to fix. Also we can do some little code cleaning up to make future mistakes less likely.
This addresses bug HIVE-2123.
https://issues.apache.org/jira/browse/HIVE-2123
Diffs (updated)
-----
trunk/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 1095838
trunk/hwi/src/java/org/apache/hadoop/hive/hwi/HWISessionItem.java 1095838
trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1095838
trunk/ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessor.java 1095838
Diff: https://reviews.apache.org/r/646/diff
Testing
-------
Thanks,
Siying
Re: Review Request: CommandNeedRetryException needs release locks and some
related code cleaning up
Posted by Siying Dong <si...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/646/#review529
-----------------------------------------------------------
trunk/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
<https://reviews.apache.org/r/646/#comment1097>
I tried to make some minimum change, but it looks much more difficult than I thought. The whole structure is built based on return codes, instead of exceptions. What I can do is that I can make Driver.run() not to throw any exception. In that case, we don't get two approaches twisted.
- Siying
On 2011-04-22 00:11:43, Siying Dong wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/646/
> -----------------------------------------------------------
>
> (Updated 2011-04-22 00:11:43)
>
>
> Review request for hive, Yongqiang He and namit jain.
>
>
> Summary
> -------
>
> now when CommandNeedRetryException is thrown, locks are not released. Not sure whether it will cause problem, since the same locks will be acquired when retrying it. It is anyway something we need to fix. Also we can do some little code cleaning up to make future mistakes less likely.
>
>
> This addresses bug HIVE-2123.
> https://issues.apache.org/jira/browse/HIVE-2123
>
>
> Diffs
> -----
>
> trunk/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 1095838
> trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1095838
>
> Diff: https://reviews.apache.org/r/646/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Siying
>
>