Description

I have an OutputFormat which implements Configurable. I set new config entries to a job configuration during checkOutputSpec() so that the tasks will get the config entries through the job configuration. This works fine in 0.20.2, but stopped working starting from 0.20.203. With 0.20.203, my OutputFormat still has the configuration set, but the copy a task gets does not have the new entries that are set as part of checkOutputSpec().

I believe that the problem is with JobClient. The job configuration needs to wait till checkOutputSpec() is returned before being cloned and submitted.

Eli Collins
added a comment - 12/Jan/12 20:36 Hi Jane,
You change looks good to me. Would you mind adding a test that would fail currently w/o your patch, and would regress if it was removed?
Thanks,
Eli

Hadoop QA
added a comment - 17/Jan/12 19:24 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12510875/mapreduce-3377-branch-1.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 3 new or modified tests.
-1 patch. The patch command could not apply the patch.
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1618//console
This message is automatically generated.

Arun C Murthy
added a comment - 17/Jan/12 21:50 Jane, currently we can only run the patch against trunk. Please paste the output of 'ant test-patch' here and also ensure 'ant test' passes. Thanks.
Also, could you please provide a patch for trunk alongwith the one for branch-1 if this issue exists there too? Thanks.

Harsh J
added a comment - 17/Jan/12 22:37 This seems to affect only the new API. The way the stable API does this remains consistent with how it used to be in 0.20.2.
Test case does fail without JC change. +1 for this patch, but we need a test case for trunk too, and if it fails without changes, a fix as well.
On 0.22+, the area of change would be under JobSubmitter#checkSpecs(…).

Jane Chen
added a comment - 18/Jan/12 04:54 No new diff introduced by the proposed change on branch-1.
The reported issue does not occur on trunk. Attached patch mapreduce-3377.patch includes only the unit test and is generated on trunk.
The other attachments shows that the added unit test passes after the proposed fix and no other diff is introduced.
out-before: stdout running "ant test" before the change.
err-before: stderr running "ant test" before the change.
out-after: stdout running "ant test" after the change.
err-after: stderr running "ant test" after the change.

Hadoop QA
added a comment - 18/Jan/12 05:55 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12510953/mapreduce-3377.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 3 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in .
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1624//testReport/
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1624//console
This message is automatically generated.

The patches are fine. I only have a few small nits, some given state of trunk today:

For trunk:

Please move the test into hadoop-mapreduce-client-jobclient project. All tests have recently been moved into that. Sorry for this inconvenience but it seems to have happened very recently, to bring them into the maven fold.

For both:

Rename test into something more appropriate, perhaps: TestMRFileOutputFormat for new API.

We could do with a test for the stable API as well, though there isn't a problem, it will help prevent regressions. You can add this test in TestFileOutputFormat, as that already targets stable API.

Harsh J
added a comment - 23/Jan/12 23:14 Hello Jane,
The patches are fine. I only have a few small nits, some given state of trunk today:
For trunk:
Please move the test into hadoop-mapreduce-client-jobclient project. All tests have recently been moved into that. Sorry for this inconvenience but it seems to have happened very recently, to bring them into the maven fold.
For both:
Rename test into something more appropriate, perhaps: TestMRFileOutputFormat for new API.
We could do with a test for the stable API as well, though there isn't a problem, it will help prevent regressions. You can add this test in TestFileOutputFormat , as that already targets stable API.
Thanks for taking the time to report and contribute!

Hadoop QA
added a comment - 24/Jan/12 02:14 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12511607/mapreduce-3377.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 3 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.mapred.TestReduceFetchFromPartialMem
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1654//testReport/
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1654//console
This message is automatically generated.

The merge to branch-1.0 included, apparently by accident, changes to build.xml that broke the native builds of task-controller and jsvc. I've reverted the build.xml change only. This error only affected branch-1.0, not branch-1 nor the other branches.

Matt Foley
added a comment - 02/May/12 23:34 The merge to branch-1.0 included, apparently by accident, changes to build.xml that broke the native builds of task-controller and jsvc. I've reverted the build.xml change only. This error only affected branch-1.0, not branch-1 nor the other branches.