-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

+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.

Hadoop QA
added a comment - 07/Mar/12 22:04 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12517467/MR-3982.txt
against trunk revision .
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.
+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.TestJobClientGetJob
org.apache.hadoop.mapred.TestMRWithDistributedCache
org.apache.hadoop.mapred.TestLocalModeWithNewApis
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2017//testReport/
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2017//console
This message is automatically generated.

The problem was that the code assumed that <OUTPUT>/_temporary/<ATTEMPT_ID>/ was being created by the individual map/reduce tasks. If it was not then other code would blowup, so instead of just creating <OUTPUT>/_temporary in the setup from before we are now creating <OUTPUT>/_temporary/<ATTEMPT_ID>/

When we clean up from a job we still want to completely remove the _temporary directory, not just <OUTPUT>/_temporary/<ATTEMPT_ID>/ or there will be an extra directory sitting around in the <OUTPUT> directory that will cause other problems.

Robert Joseph Evans
added a comment - 08/Mar/12 16:44 That would not really work. The directory structure is the following.
<OUTPUT>/_temporary/<ATTEMPT_ID>/...
The problem was that the code assumed that <OUTPUT>/_temporary/<ATTEMPT_ID>/ was being created by the individual map/reduce tasks. If it was not then other code would blowup, so instead of just creating <OUTPUT>/_temporary in the setup from before we are now creating <OUTPUT>/_temporary/<ATTEMPT_ID>/
When we clean up from a job we still want to completely remove the _temporary directory, not just <OUTPUT>/_temporary/<ATTEMPT_ID>/ or there will be an extra directory sitting around in the <OUTPUT> directory that will cause other problems.

This patch adds in a unit test that verifies that it still works even without any output.

I started to port TestEmptyJob to the MiniMrYarnCluster, but it uses a lot of methods that are specific to the JT implementation to do some sort of synchronization test. I really don't understand all of the reasons for that test and why it is trying to force the scheduler to schedule something. I will file a separate JIRA to port it over because it is probably going to take a while for me to fully understand the test.

Robert Joseph Evans
added a comment - 09/Mar/12 14:55 This patch adds in a unit test that verifies that it still works even without any output.
I started to port TestEmptyJob to the MiniMrYarnCluster, but it uses a lot of methods that are specific to the JT implementation to do some sort of synchronization test. I really don't understand all of the reasons for that test and why it is trying to force the scheduler to schedule something. I will file a separate JIRA to port it over because it is probably going to take a while for me to fully understand the test.

Hadoop QA
added a comment - 09/Mar/12 15:57 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12517725/MR-3982.txt
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/2025//testReport/
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2025//console
This message is automatically generated.