You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Ryota Egashira <eg...@yahoo-inc.com> on 2014/08/26 19:11:18 UTC
Re: Review Request 25066: OOZIE-1728 When an ApplicationMaster
restarts, it restarts the launcher job: DistCp followup
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25066/
-----------------------------------------------------------
(Updated Aug. 26, 2014, 5:11 p.m.)
Review request for oozie.
Changes
-------
uploaded wrong version previously
Bugs: OOZIE-1728
https://issues.apache.org/jira/browse/OOZIE-1728
Repository: oozie-git
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1728
Diffs (updated)
-----
core/pom.xml 5b2eedc
core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java fe31d7b
core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java PRE-CREATION
core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java PRE-CREATION
sharelib/distcp/pom.xml 04e436d
sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java PRE-CREATION
sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java b075957
Diff: https://reviews.apache.org/r/25066/diff/
Testing
-------
upload patch for review, doing e2e test now.
Thanks,
Ryota Egashira
Re: Review Request 25066: OOZIE-1728 When an ApplicationMaster
restarts, it restarts the launcher job: DistCp followup
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
> On Aug. 26, 2014, 11:24 p.m., Robert Kanter wrote:
> > sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java, line 81
> > <https://reviews.apache.org/r/25066/diff/2/?file=669441#file669441line81>
> >
> > Shouldn't we be setting constArgs[1] to the DistCpOptions? Even if we're passing null, it would be best to explicetly set it (i.e. constArgs[1] = null;)
we pass args when invoke run(args), which is parsed by OptionsParser, so null is fine at the time of instantiating.
explicitly added constArgs[1] = null.
- Ryota
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25066/#review51607
-----------------------------------------------------------
On Aug. 26, 2014, 11:32 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25066/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2014, 11:32 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1728
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Diffs
> -----
>
> core/pom.xml 5b2eedc
> core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java fe31d7b
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java PRE-CREATION
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java PRE-CREATION
> sharelib/distcp/pom.xml 04e436d
> sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java PRE-CREATION
> sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java b075957
>
> Diff: https://reviews.apache.org/r/25066/diff/
>
>
> Testing
> -------
>
> upload patch for review, doing e2e test now.
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 25066: OOZIE-1728 When an ApplicationMaster
restarts, it restarts the launcher job: DistCp followup
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25066/#review51607
-----------------------------------------------------------
Looks good overall, other than some minor things.
Also, please make sure to run unit tests (and if possible, e2e testing) with Hadoop 1.x, 2.x, and 0.23.x and both DistCp v1 and v2 where applicable.
core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java
<https://reviews.apache.org/r/25066/#comment90053>
@Override
sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java
<https://reviews.apache.org/r/25066/#comment90054>
I think you can get rid of the "throwing exception" part of the message, that's pretty obvious :)
sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java
<https://reviews.apache.org/r/25066/#comment90055>
Shouldn't this be break? Otherwise, it will just go to the next constructor in the loop.
sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java
<https://reviews.apache.org/r/25066/#comment90059>
Shouldn't we be setting constArgs[1] to the DistCpOptions? Even if we're passing null, it would be best to explicetly set it (i.e. constArgs[1] = null;)
sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java
<https://reviews.apache.org/r/25066/#comment90056>
Same as the other; it should be break.
- Robert Kanter
On Aug. 26, 2014, 5:11 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25066/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2014, 5:11 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1728
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Diffs
> -----
>
> core/pom.xml 5b2eedc
> core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java fe31d7b
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java PRE-CREATION
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java PRE-CREATION
> sharelib/distcp/pom.xml 04e436d
> sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java PRE-CREATION
> sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java b075957
>
> Diff: https://reviews.apache.org/r/25066/diff/
>
>
> Testing
> -------
>
> upload patch for review, doing e2e test now.
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 25066: OOZIE-1728 When an ApplicationMaster
restarts, it restarts the launcher job: DistCp followup
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25066/#review54200
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Kanter
On Sept. 15, 2014, 11:20 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25066/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2014, 11:20 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1728
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Diffs
> -----
>
> core/pom.xml 59b6ecd
> core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java 86d21fb
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java PRE-CREATION
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java PRE-CREATION
> sharelib/distcp/pom.xml 04e436d
> sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java PRE-CREATION
> sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java 7a098f3
>
> Diff: https://reviews.apache.org/r/25066/diff/
>
>
> Testing
> -------
>
> upload patch for review, doing e2e test now.
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 25066: OOZIE-1728 When an ApplicationMaster
restarts, it restarts the launcher job: DistCp followup
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25066/
-----------------------------------------------------------
(Updated Sept. 15, 2014, 11:20 p.m.)
Review request for oozie.
Changes
-------
fixed @override, added logic of kerberos credential copy.
verified in e2e test.
Bugs: OOZIE-1728
https://issues.apache.org/jira/browse/OOZIE-1728
Repository: oozie-git
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1728
Diffs (updated)
-----
core/pom.xml 59b6ecd
core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java 86d21fb
core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java PRE-CREATION
core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java PRE-CREATION
sharelib/distcp/pom.xml 04e436d
sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java PRE-CREATION
sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java 7a098f3
Diff: https://reviews.apache.org/r/25066/diff/
Testing
-------
upload patch for review, doing e2e test now.
Thanks,
Ryota Egashira
Re: Review Request 25066: OOZIE-1728 When an ApplicationMaster
restarts, it restarts the launcher job: DistCp followup
Posted by Robert Kanter <rk...@cloudera.com>.
> On Aug. 29, 2014, 9:01 p.m., Robert Kanter wrote:
> > Assuming the e2e tests pass, LGTM. Can you also add the missing @Override I mentioned?
btw, I just pushed in the change to Hadoop 1.2.1 -- I'm not sure if that will affect anything here.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25066/#review51908
-----------------------------------------------------------
On Aug. 26, 2014, 11:32 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25066/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2014, 11:32 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1728
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Diffs
> -----
>
> core/pom.xml 5b2eedc
> core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java fe31d7b
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java PRE-CREATION
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java PRE-CREATION
> sharelib/distcp/pom.xml 04e436d
> sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java PRE-CREATION
> sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java b075957
>
> Diff: https://reviews.apache.org/r/25066/diff/
>
>
> Testing
> -------
>
> upload patch for review, doing e2e test now.
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 25066: OOZIE-1728 When an ApplicationMaster
restarts, it restarts the launcher job: DistCp followup
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25066/#review51908
-----------------------------------------------------------
Ship it!
Assuming the e2e tests pass, LGTM. Can you also add the missing @Override I mentioned?
- Robert Kanter
On Aug. 26, 2014, 11:32 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25066/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2014, 11:32 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1728
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1728
>
>
> Diffs
> -----
>
> core/pom.xml 5b2eedc
> core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java fe31d7b
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java PRE-CREATION
> core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java PRE-CREATION
> sharelib/distcp/pom.xml 04e436d
> sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java PRE-CREATION
> sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java b075957
>
> Diff: https://reviews.apache.org/r/25066/diff/
>
>
> Testing
> -------
>
> upload patch for review, doing e2e test now.
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 25066: OOZIE-1728 When an ApplicationMaster
restarts, it restarts the launcher job: DistCp followup
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25066/
-----------------------------------------------------------
(Updated Aug. 26, 2014, 11:32 p.m.)
Review request for oozie.
Changes
-------
modifeid comments, thanks robert
Bugs: OOZIE-1728
https://issues.apache.org/jira/browse/OOZIE-1728
Repository: oozie-git
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1728
Diffs (updated)
-----
core/pom.xml 5b2eedc
core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java fe31d7b
core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java PRE-CREATION
core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java PRE-CREATION
sharelib/distcp/pom.xml 04e436d
sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java PRE-CREATION
sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java b075957
Diff: https://reviews.apache.org/r/25066/diff/
Testing
-------
upload patch for review, doing e2e test now.
Thanks,
Ryota Egashira