MAPREDUCE-838. Fixes a problem in the way commit of task outputs happens. The bug was that even if commit failed, the task would be declared as successful. Contributed by Amareshwari Sriramadasu.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-0.20@802999 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -201,6 +201,10 @@ Release 0.20.1 - Unreleased
 
     HDFS-525. The SimpleDateFormat object in ListPathsServlet is not thread
     safe.  (Suresh Srinivas via szetszwo)
+
+    MAPREDUCE-838. Fixes a problem in the way commit of task outputs
+    happens. The bug was that even if commit failed, the task would
+    be declared as successful. (Amareshwari Sriramadasu via ddas)
  
 Release 0.20.0 - 2009-04-15
 

src/mapred/org/apache/hadoop/mapred/Task.java

@@ -742,27 +742,30 @@ abstract class Task implements Writable, Configurable {
           }
           reporter.setProgressFlag();
         }
-        // task can Commit now  
-        try {
-          LOG.info("Task " + taskId + " is allowed to commit now");
-          committer.commitTask(taskContext);
-          return;
-        } catch (IOException iee) {
-          LOG.warn("Failure committing: " + 
-                    StringUtils.stringifyException(iee));
-          discardOutput(taskContext);
-          throw iee;
-        }
+        break;
       } catch (IOException ie) {
         LOG.warn("Failure asking whether task can commit: " + 
             StringUtils.stringifyException(ie));
         if (--retries == 0) {
-          //if it couldn't commit a successfully then delete the output
+          //if it couldn't query successfully then delete the output
           discardOutput(taskContext);
           System.exit(68);
         }
       }
     }
+    
+    // task can Commit now  
+    try {
+      LOG.info("Task " + taskId + " is allowed to commit now");
+      committer.commitTask(taskContext);
+      return;
+    } catch (IOException iee) {
+      LOG.warn("Failure committing: " + 
+        StringUtils.stringifyException(iee));
+      //if it couldn't commit a successfully then delete the output
+      discardOutput(taskContext);
+      throw iee;
+    }
   }
 
   private 

src/test/org/apache/hadoop/mapred/TestTaskCommit.java

@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.mapred;
+
+import java.io.IOException;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+public class TestTaskCommit extends HadoopTestCase {
+
+  static class CommitterWithCommitFail extends FileOutputCommitter {
+    public void commitTask(TaskAttemptContext context) throws IOException {
+      Path taskOutputPath = getTempTaskOutputPath(context);
+      TaskAttemptID attemptId = context.getTaskAttemptID();
+      JobConf job = context.getJobConf();
+      if (taskOutputPath != null) {
+        FileSystem fs = taskOutputPath.getFileSystem(job);
+        if (fs.exists(taskOutputPath)) {
+          throw new IOException();
+        }
+      }
+    }
+  }
+
+  public TestTaskCommit() throws IOException {
+    super(LOCAL_MR, LOCAL_FS, 1, 1);
+  }
+  
+  public void testCommitFail() throws IOException {
+    Path rootDir = 
+      new Path(System.getProperty("test.build.data",  "/tmp"), "test");
+    final Path inDir = new Path(rootDir, "input");
+    final Path outDir = new Path(rootDir, "output");
+    JobConf jobConf = createJobConf();
+    jobConf.setMaxMapAttempts(1);
+    jobConf.setOutputCommitter(CommitterWithCommitFail.class);
+    RunningJob rJob = UtilsForTests.runJob(jobConf, inDir, outDir, 1, 0);
+    rJob.waitForCompletion();
+    assertEquals(JobStatus.FAILED, rJob.getJobState());
+  }
+
+  public static void main(String[] argv) throws Exception {
+    TestTaskCommit td = new TestTaskCommit();
+    td.testCommitFail();
+  }
+}