You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Mona Chitnis <mo...@yahoo.in> on 2013/08/08 03:23:14 UTC
Review Request 13397: OOZIE-1486 Cut down on number of small files created
to track a running action
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/
-----------------------------------------------------------
Review request for oozie.
Bugs: OOZIE-1486
https://issues.apache.org/jira/browse/OOZIE-1486
Repository: oozie
Description
-------
See JIRA Description
Diffs
-----
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1511528
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1511528
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1511528
trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1511528
trunk/core/src/main/java/org/apache/oozie/util/IOUtils.java 1511528
trunk/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java 1511528
trunk/core/src/main/java/org/apache/oozie/util/StatusUtils.java 1511528
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1511528
trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1511528
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1511528
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1511528
trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMain.java 1511528
trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1511528
Diff: https://reviews.apache.org/r/13397/diff/
Testing
-------
Existing unit tests should pass, and E-2-E test pending.
Few tests failing with a common problem of Properties object (for output/new-id/error etc) getting stored in the file with Timestamp. Hence on retrieval, it gives NPE on the expected keys. WIP to fix that small part
Thanks,
Mona Chitnis
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/#review25092
-----------------------------------------------------------
Leave the action Main methods writing data to local files as is. In the LauncherMapper map() finally, read from the multiple files and write to one sequence file.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49297>
Remove this method from all action executors and handle the logic only in super class.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49296>
Remove this method altogether. Was added in OOZIE-1160, but redundant. Can be done with getActionData() itself if we need to do modify context.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49291>
To be removed
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49302>
Load the sequence file into a map here using a readActionOutput method if running job is complete
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49299>
Check if the id is there in the loaded map
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49301>
Load the sequence file into a map here using a readActionOutput method if running job is complete and set the following based on the contents of the map<String, String>.
context.setErrorInfo
context.setExecutionData
context.setExecutionStats
context.setExternalChildIDs
Sequencefile read will use the same Text object. So convert to String and store in the map
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49303>
If id was null in the sequence file, use this method to only check from the counters
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49309>
getActionOutputPath or getActionDataPath
trunk/core/src/main/java/org/apache/oozie/util/IOUtils.java
<https://reviews.apache.org/r/13397/#comment49306>
Can be removed
trunk/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java
<https://reviews.apache.org/r/13397/#comment49307>
Can be removed or reverted.
trunk/core/src/main/java/org/apache/oozie/util/StatusUtils.java
<https://reviews.apache.org/r/13397/#comment49308>
Change not related. Can we revert this? If not you can add more info to javadoc instead of changing the method signature.
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49304>
Please use .seq extension for sequence file. Something like actionData.seq or actionOut.seq. Also the helper method names should refer to sequence file instead of properties.
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49305>
Use a new or retain the action prefix, so that the variables are grouped.
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49287>
Extract prepare xml and pass instead of changing the PrepareActionsDriver.doOperations api to actionXml. Leave the PrepareActionsDriver.doOperations as it is.
- Rohini Palaniswamy
On Aug. 13, 2013, 5:14 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13397/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2013, 5:14 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1486
> https://issues.apache.org/jira/browse/OOZIE-1486
>
>
> Repository: oozie
>
>
> Description
> -------
>
> See JIRA Description
>
>
> Diffs
> -----
>
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1513230
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1513230
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1513230
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1513230
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1513230
> trunk/core/src/main/java/org/apache/oozie/util/IOUtils.java 1513230
> trunk/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java 1513230
> trunk/core/src/main/java/org/apache/oozie/util/StatusUtils.java 1513230
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 1513230
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1513230
> trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1513230
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1513230
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1513230
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/PrepareActionsDriver.java 1513230
> trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMain.java 1513230
> trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1513230
>
> Diff: https://reviews.apache.org/r/13397/diff/
>
>
> Testing
> -------
>
> Existing unit tests should pass, and E-2-E test pending.
>
> Few tests failing with a common problem of Properties object (for output/new-id/error etc) getting stored in the file with Timestamp. Hence on retrieval, it gives NPE on the expected keys. WIP to fix that small part
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/#review25309
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49665>
Can you put this inside a null check block
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49666>
Do this reload only if actionData.containsKey(LauncherMapper.ACTION_DATA_NEW_ID)
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49667>
Why separately for sqoop? Should be fixed to be same for all. Fix SqoopMain
Do not set childjob ids again in this block.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49675>
Remove runningjob from the API
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49668>
Do not check counter. Check if actionData containskey
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49669>
Lets do counter for isMainSuccessful alone.
Map<String, String>
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49676>
Remove runningjob
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49670>
local variable for Path for getActionData..
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49673>
Still need this counter. Lets just retain this one
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49672>
commons-io is not in hadoop classpath in Hadoop 1.x
- Rohini Palaniswamy
On Aug. 19, 2013, 7:40 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13397/
> -----------------------------------------------------------
>
> (Updated Aug. 19, 2013, 7:40 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1486
> https://issues.apache.org/jira/browse/OOZIE-1486
>
>
> Repository: oozie
>
>
> Description
> -------
>
> See JIRA Description
>
>
> Diffs
> -----
>
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1514596
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1514596
> trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1514596
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1514596
>
> Diff: https://reviews.apache.org/r/13397/diff/
>
>
> Testing
> -------
>
> Existing unit tests should pass, and E-2-E test pending.
>
> Few tests failing with a common problem of Properties object (for output/new-id/error etc) getting stored in the file with Timestamp. Hence on retrieval, it gives NPE on the expected keys. WIP to fix that small part
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/#review25370
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49760>
Should not be removed.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49761>
Add a comment saying fetching action output and stats for the Mapreduce action. Would make it clear for anyone working on id swap removal.
trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49762>
Can we retain the assert for external child ids?
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49763>
Pass the actual exception also.
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49764>
You also need to check for external child ids and store them on a action failure. Just not on errormessage == null
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java
<https://reviews.apache.org/r/13397/#comment49768>
You can just use FileOutputStream since it is only one line similar to the change done in SqoopMain
trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49765>
can we have the equivalent assert added and also one for external child ids?
trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49766>
can we have the equivalent assert added and also one for external child ids?
trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java
<https://reviews.apache.org/r/13397/#comment49767>
Can we use the constant in LauncherMapper here?
trunk/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49769>
Would still be good to retain these asserts to catch any unexpected change till we remove id swap all together.
- Rohini Palaniswamy
On Aug. 21, 2013, 1:50 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13397/
> -----------------------------------------------------------
>
> (Updated Aug. 21, 2013, 1:50 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1486
> https://issues.apache.org/jira/browse/OOZIE-1486
>
>
> Repository: oozie
>
>
> Description
> -------
>
> See JIRA Description
>
>
> Diffs
> -----
>
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1516008
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1516008
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1516008
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1516008
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1516008
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java 1516008
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1516008
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java 1516008
> trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionCheckXCommand.java 1516008
> trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java 1516008
> trunk/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java 1516008
> trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1516008
> trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java 1516008
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1516008
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1516008
> trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java 1516008
> trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1516008
> trunk/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java 1516008
> trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java 1516008
>
> Diff: https://reviews.apache.org/r/13397/diff/
>
>
> Testing
> -------
>
> E-2-E test with Pig stats verified.
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/#review25429
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49837>
It is covered already in the function that follows - context.setExternalData - internally also calls action.setExternalStatus
trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java
<https://reviews.apache.org/r/13397/#comment49838>
Then makes sense to make change in all places where strings are used
trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java
<https://reviews.apache.org/r/13397/#comment49839>
Then makes sense to make the change across all files using strings for these
- Mona Chitnis
On Aug. 21, 2013, 1:50 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13397/
> -----------------------------------------------------------
>
> (Updated Aug. 21, 2013, 1:50 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1486
> https://issues.apache.org/jira/browse/OOZIE-1486
>
>
> Repository: oozie
>
>
> Description
> -------
>
> See JIRA Description
>
>
> Diffs
> -----
>
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1516008
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1516008
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1516008
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1516008
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1516008
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java 1516008
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1516008
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java 1516008
> trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionCheckXCommand.java 1516008
> trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java 1516008
> trunk/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java 1516008
> trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1516008
> trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java 1516008
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1516008
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1516008
> trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java 1516008
> trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1516008
> trunk/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java 1516008
> trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java 1516008
>
> Diff: https://reviews.apache.org/r/13397/diff/
>
>
> Testing
> -------
>
> E-2-E test with Pig stats verified.
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/#review25478
-----------------------------------------------------------
Just two more minor comments.
trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49909>
Add a assert not null check. If both are null also then assertEquals will be true.
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49910>
Can remove this. You don't have to upload again here as you are doing it in map finally. It will be overwritten again in finally.
- Rohini Palaniswamy
On Aug. 22, 2013, 11:49 p.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13397/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2013, 11:49 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1486
> https://issues.apache.org/jira/browse/OOZIE-1486
>
>
> Repository: oozie
>
>
> Description
> -------
>
> See JIRA Description
>
>
> Diffs
> -----
>
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1516594
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1516594
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1516594
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1516594
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1516594
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 1516594
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1516594
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceMain.java 1516594
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPipesMain.java 1516594
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java 1516594
> trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionCheckXCommand.java 1516594
> trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java 1516594
> trunk/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java 1516594
> trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1516594
> trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java 1516594
> trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveMain.java 1516594
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1516594
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1516594
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java 1516594
> trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMain.java 1516594
> trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMainWithOldAPI.java 1516594
> trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java 1516594
> trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigMain.java 1516594
> trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1516594
> trunk/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java 1516594
> trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java 1516594
> trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestStreamingMain.java 1516594
>
> Diff: https://reviews.apache.org/r/13397/diff/
>
>
> Testing
> -------
>
> E-2-E test with Pig stats verified.
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/
-----------------------------------------------------------
(Updated Aug. 26, 2013, 8:41 p.m.)
Review request for oozie.
Changes
-------
final round of comments addressed and failing unit test from Jenkins fixed
Bugs: OOZIE-1486
https://issues.apache.org/jira/browse/OOZIE-1486
Repository: oozie
Description
-------
See JIRA Description
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1516936
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1516936
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1516936
trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1516936
trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1516936
trunk/core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 1516936
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1516936
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceMain.java 1516936
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPipesMain.java 1516936
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java 1516936
trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionCheckXCommand.java 1516936
trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java 1516936
trunk/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java 1516936
trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1516936
trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java 1516936
trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveMain.java 1516936
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1516936
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1516936
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java 1516936
trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMain.java 1516936
trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMainWithOldAPI.java 1516936
trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java 1516936
trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigMain.java 1516936
trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1516936
trunk/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java 1516936
trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java 1516936
trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestStreamingMain.java 1516936
Diff: https://reviews.apache.org/r/13397/diff/
Testing
-------
E-2-E test with Pig stats verified.
Thanks,
Mona Chitnis
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/
-----------------------------------------------------------
(Updated Aug. 22, 2013, 11:49 p.m.)
Review request for oozie.
Changes
-------
Addressed comments.
FYI - HiveMain and SqoopMain have been changed to store the child Hadoop Job IDs under context.externalChildIDs. Earlier they were being stored redundantly as context.externalChildIDs as well as context.action.data (output.properties). Hence some change reflected in Main classes as well as the unit tests
Bugs: OOZIE-1486
https://issues.apache.org/jira/browse/OOZIE-1486
Repository: oozie
Description
-------
See JIRA Description
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1516594
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1516594
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1516594
trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1516594
trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1516594
trunk/core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 1516594
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1516594
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceMain.java 1516594
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPipesMain.java 1516594
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java 1516594
trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionCheckXCommand.java 1516594
trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java 1516594
trunk/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java 1516594
trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1516594
trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java 1516594
trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveMain.java 1516594
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1516594
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1516594
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java 1516594
trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMain.java 1516594
trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMainWithOldAPI.java 1516594
trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java 1516594
trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigMain.java 1516594
trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1516594
trunk/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java 1516594
trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java 1516594
trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestStreamingMain.java 1516594
Diff: https://reviews.apache.org/r/13397/diff/
Testing
-------
E-2-E test with Pig stats verified.
Thanks,
Mona Chitnis
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/
-----------------------------------------------------------
(Updated Aug. 21, 2013, 1:50 a.m.)
Review request for oozie.
Changes
-------
Previous patch was missing some files.
Bugs: OOZIE-1486
https://issues.apache.org/jira/browse/OOZIE-1486
Repository: oozie
Description
-------
See JIRA Description
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1516008
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1516008
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1516008
trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1516008
trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1516008
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java 1516008
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1516008
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java 1516008
trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionCheckXCommand.java 1516008
trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java 1516008
trunk/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java 1516008
trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1516008
trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java 1516008
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1516008
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1516008
trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java 1516008
trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1516008
trunk/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java 1516008
trunk/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java 1516008
Diff: https://reviews.apache.org/r/13397/diff/
Testing (updated)
-------
E-2-E test with Pig stats verified.
Thanks,
Mona Chitnis
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/
-----------------------------------------------------------
(Updated Aug. 19, 2013, 7:40 a.m.)
Review request for oozie.
Changes
-------
unit tests Pig/Hive/Sqoop involving externalIds and outputdata are failing. Reasons
1. data in mem obj might be pertaining to launcher job and not of the actual action.
2. Reading back sequencefile Text value into String, not delimiting correctly e.g. in error-properties, error.message and error.code are two properties but is getting retrieved as one single string.
Bugs: OOZIE-1486
https://issues.apache.org/jira/browse/OOZIE-1486
Repository: oozie
Description
-------
See JIRA Description
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1514596
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1514596
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1514596
trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1514596
trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1514596
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1514596
trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1514596
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1514596
Diff: https://reviews.apache.org/r/13397/diff/
Testing
-------
Existing unit tests should pass, and E-2-E test pending.
Few tests failing with a common problem of Properties object (for output/new-id/error etc) getting stored in the file with Timestamp. Hence on retrieval, it gives NPE on the expected keys. WIP to fix that small part
Thanks,
Mona Chitnis
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Mona Chitnis <mo...@yahoo.in>.
> On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java, lines 1030-1034
> > <https://reviews.apache.org/r/13397/diff/3/?file=341729#file341729line1030>
> >
> > Don't have to make hasIdSwap call and check the counter. You can just check if the map has the key. Please create another jira to remove code related to setting and checking of the counter or if possible do it in this jira itself.
> On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java, line 1066
> > <https://reviews.apache.org/r/13397/diff/3/?file=341729#file341729line1066>
> >
> > If there was an id swap, i think you need to reload the action data here.
hmm think so too.
> On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java, line 277
> > <https://reviews.apache.org/r/13397/diff/3/?file=341730#file341730line277>
> >
> > Do a file exists check and return empty map if the file does not exist. The file will not exist if the launcher did not run at all.
> >
> > There should be no fs.getHomeDirectory(). The path should just be getActionDataSequenceFilePath(actionDir)
ah. maybe thats why i was getting filenotfound error
> On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote:
> > trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java, line 85
> > <https://reviews.apache.org/r/13397/diff/3/?file=341735#file341735line85>
> >
> > The file name will become oozie.action.data.seq. Just have action-data.seq or action_data.seq. No . in the middle of the file name.
this is what happens when one burns the midnight oil :/
> On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote:
> > trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java, lines 347-357
> > <https://reviews.apache.org/r/13397/diff/3/?file=341735#file341735line347>
> >
> > This should be in a finally block after LauncherException catch block. You are only creating the file in case of an exception now.
same as above.
> On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote:
> > trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java, line 435
> > <https://reviews.apache.org/r/13397/diff/3/?file=341735#file341735line435>
> >
> > You don't need hadoop filesystem to read a File. Just read using BufferedInputStream and FileInputStream. Also do not read line by line. Read as is into byte array. Currently you are reading line by line and appending to stringbuffer without appending \n.
> >
> > Can do FileUtils.readFileToString() from commons-io package, but commons-io is not present in hadoop 1.x. Only in 0.23.
okay thanks
- Mona
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/#review25280
-----------------------------------------------------------
On Aug. 16, 2013, 9:31 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13397/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2013, 9:31 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1486
> https://issues.apache.org/jira/browse/OOZIE-1486
>
>
> Repository: oozie
>
>
> Description
> -------
>
> See JIRA Description
>
>
> Diffs
> -----
>
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1514596
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1514596
> trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1514596
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1514596
>
> Diff: https://reviews.apache.org/r/13397/diff/
>
>
> Testing
> -------
>
> Existing unit tests should pass, and E-2-E test pending.
>
> Few tests failing with a common problem of Properties object (for output/new-id/error etc) getting stored in the file with Timestamp. Hence on retrieval, it gives NPE on the expected keys. WIP to fix that small part
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/#review25280
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49566>
Why remove private?
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49576>
This can be a local variable inside check() method. Don't need a class variable.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49569>
Avoid passing objects and modifying them in methods. Make the method return a map as the return type.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49574>
Don't have to make hasIdSwap call and check the counter. You can just check if the map has the key. Please create another jira to remove code related to setting and checking of the counter or if possible do it in this jira itself.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49567>
Can store the id directly as String instead of having Properties.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49571>
If there was an id swap, i think you need to reload the action data here.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/13397/#comment49570>
Remove the hasOutputData and hasStatsData checks. containsKey should be good enough
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49575>
getActionData(..)
Please make the method return the map. Do not take it as argument and put values in it. Not a good practice.
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49577>
Do a file exists check and return empty map if the file does not exist. The file will not exist if the launcher did not run at all.
There should be no fs.getHomeDirectory(). The path should just be getActionDataSequenceFilePath(actionDir)
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49578>
This is not required. Just do new Text().
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
<https://reviews.apache.org/r/13397/#comment49579>
Null check not required. If any of it was null reading the file would fail
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java
<https://reviews.apache.org/r/13397/#comment49581>
Can be removed.
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java
<https://reviews.apache.org/r/13397/#comment49582>
Why remove this?
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java
<https://reviews.apache.org/r/13397/#comment49583>
assertFalse(fs.exists(LauncherMapperHelper.getActionDataSequenceFilePath(actionDir)));
Same in other places
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java
<https://reviews.apache.org/r/13397/#comment49580>
To be cleaned?
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49572>
The file name will become oozie.action.data.seq. Just have action-data.seq or action_data.seq. No . in the middle of the file name.
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49585>
This should be in a finally block after LauncherException catch block. You are only creating the file in case of an exception now.
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49584>
Path finalPath = new Path(actionDir, ACTION_DATA_SEQUENCE_FILE);
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49586>
You don't need hadoop filesystem to read a File. Just read using BufferedInputStream and FileInputStream. Also do not read line by line. Read as is into byte array. Currently you are reading line by line and appending to stringbuffer without appending \n.
Can do FileUtils.readFileToString() from commons-io package, but commons-io is not present in hadoop 1.x. Only in 0.23.
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/13397/#comment49573>
It should be prepareXML. actionXml is the file that contains the action configuration.
- Rohini Palaniswamy
On Aug. 16, 2013, 9:31 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13397/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2013, 9:31 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1486
> https://issues.apache.org/jira/browse/OOZIE-1486
>
>
> Repository: oozie
>
>
> Description
> -------
>
> See JIRA Description
>
>
> Diffs
> -----
>
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1514596
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1514596
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1514596
> trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1514596
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1514596
>
> Diff: https://reviews.apache.org/r/13397/diff/
>
>
> Testing
> -------
>
> Existing unit tests should pass, and E-2-E test pending.
>
> Few tests failing with a common problem of Properties object (for output/new-id/error etc) getting stored in the file with Timestamp. Hence on retrieval, it gives NPE on the expected keys. WIP to fix that small part
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/
-----------------------------------------------------------
(Updated Aug. 16, 2013, 9:31 a.m.)
Review request for oozie.
Changes
-------
Addressed Rohini's review comments. testLauncher unit test WIP. Wasnt able to figure out why loading sequence file is problematic sometimes
Bugs: OOZIE-1486
https://issues.apache.org/jira/browse/OOZIE-1486
Repository: oozie
Description
-------
See JIRA Description
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1514596
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1514596
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1514596
trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1514596
trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1514596
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1514596
trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1514596
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1514596
Diff: https://reviews.apache.org/r/13397/diff/
Testing
-------
Existing unit tests should pass, and E-2-E test pending.
Few tests failing with a common problem of Properties object (for output/new-id/error etc) getting stored in the file with Timestamp. Hence on retrieval, it gives NPE on the expected keys. WIP to fix that small part
Thanks,
Mona Chitnis
Re: Review Request 13397: OOZIE-1486 Cut down on number of small files
created to track a running action
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13397/
-----------------------------------------------------------
(Updated Aug. 13, 2013, 5:14 a.m.)
Review request for oozie.
Changes
-------
1. Converted text-based file to store all action properties, into SequenceFile<Text,Text> where key=type i.e. output.properties/newId/stat/error etc and value is the properties/string.
2. Removed another redundant file action_prepare_xml by utilizing action_conf_xml itself.
All unit tests pass except 4 in JavaActionExecutor because Properties.load(Text.toString()) is not separating on proper key=value delimiters. Minor issue which I'll debug in a day and shouldn't change the logic much
Bugs: OOZIE-1486
https://issues.apache.org/jira/browse/OOZIE-1486
Repository: oozie
Description
-------
See JIRA Description
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 1513230
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1513230
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java 1513230
trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 1513230
trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 1513230
trunk/core/src/main/java/org/apache/oozie/util/IOUtils.java 1513230
trunk/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java 1513230
trunk/core/src/main/java/org/apache/oozie/util/StatusUtils.java 1513230
trunk/core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 1513230
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 1513230
trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 1513230
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 1513230
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 1513230
trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/PrepareActionsDriver.java 1513230
trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMain.java 1513230
trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java 1513230
Diff: https://reviews.apache.org/r/13397/diff/
Testing
-------
Existing unit tests should pass, and E-2-E test pending.
Few tests failing with a common problem of Properties object (for output/new-id/error etc) getting stored in the file with Timestamp. Hence on retrieval, it gives NPE on the expected keys. WIP to fix that small part
Thanks,
Mona Chitnis