commit 6dd534ecac70adb61233f55ce728261de2ea26bc

Author: Hemanth Yamijala <yhemanth@yahoo-inc.com>
Date:   Thu Dec 17 01:18:36 2009 +0530

    MAPREDUCE:896 Additional patch to fix spurious logging in tasktracker logs from https://issues.apache.org/jira/secure/attachment/12428180/y896.v2.1.fix.v2.patch
    
    +++ b/YAHOO-CHANGES.txt
    +    MAPREDUCE-896. Fix bug in earlier implementation to prevent
    +    spurious logging in tasktracker logs for absent file paths.
    +    (Ravi Gummadi via yhemanth)
    +


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

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

@@ -88,7 +88,10 @@ class CleanupQueue {
     if (LOG.isDebugEnabled()) {
       LOG.debug("Trying to delete " + context.fullPath);
     }
-    return context.fs.delete(new Path(context.fullPath), true);
+    if (context.fs.exists(new Path(context.fullPath))) {
+      return context.fs.delete(new Path(context.fullPath), true);
+    }
+    return true;
   }
 
   private static class PathCleanupThread extends Thread {

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

@@ -147,6 +147,8 @@ class DefaultTaskController extends TaskController {
     } catch(InterruptedException e) {
       LOG.warn("Interrupted while setting permissions for " + context.fullPath +
           " for deletion.");
+    } catch(IOException ioe) {
+      LOG.warn("Unable to change permissions of " + context.fullPath);
     }
   }
 }

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

@@ -265,10 +265,15 @@ class LinuxTaskController extends TaskController {
       TaskControllerPathDeletionContext tContext =
         (TaskControllerPathDeletionContext) context;
     
-      if (tContext.task.getUser() != null && tContext.fs instanceof LocalFileSystem) {
-        runCommand(TaskCommands.ENABLE_TASK_FOR_CLEANUP,
+      if (tContext.task.getUser() != null &&
+          tContext.fs instanceof LocalFileSystem) {
+        try {
+          runCommand(TaskCommands.ENABLE_TASK_FOR_CLEANUP,
                    tContext.task.getUser(),
                    buildTaskCleanupArgs(tContext), null, null);
+        } catch(IOException e) {
+          LOG.warn("Uanble to change permissions for " + tContext.fullPath);
+        }
       }
       else {
         throw new IllegalArgumentException("Either user is null or the "  +

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

@@ -179,7 +179,9 @@ abstract class TaskController implements Configurable {
     @Override
     protected void enablePathForCleanup() throws IOException {
       getPathForCleanup();// allow init of fullPath
-      taskController.enableTaskForCleanup(this);
+      if (fs.exists(new Path(fullPath))) {
+        taskController.enableTaskForCleanup(this); 
+      }
     }
   }
 

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

@@ -552,7 +552,6 @@ abstract class TaskRunner extends Thread {
   }
   
   /**
-   * Sets permissions recursively and then deletes the contents of dir.
    * Makes dir empty directory(does not delete dir itself).
    */
   static void deleteDirContents(JobConf conf, File dir) throws IOException {
@@ -561,18 +560,6 @@ abstract class TaskRunner extends Thread {
       File contents[] = dir.listFiles();
       if (contents != null) {
         for (int i = 0; i < contents.length; i++) {
-          try {
-            int ret = 0;
-            if ((ret = FileUtil.chmod(contents[i].getAbsolutePath(),
-                                      "a+rwx", true)) != 0) {
-              LOG.warn("Unable to chmod for " + contents[i] + 
-                  "; chmod exit status = " + ret);
-            }
-          } catch(InterruptedException e) {
-            LOG.warn("Interrupted while setting permissions for contents of " +
-                "workDir. Not deleting the remaining contents of workDir.");
-            return;
-          }
           if (!fs.delete(new Path(contents[i].getAbsolutePath()), true)) {
             LOG.warn("Unable to delete "+ contents[i]);
           }

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

@@ -23,45 +23,36 @@ import java.io.IOException;
 
 import junit.framework.TestCase;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.fs.Path;
 
+/**
+ * Validates if TaskRunner.deleteDirContents() is properly cleaning up the
+ * contents of workDir.
+ */
 public class TestSetupWorkDir extends TestCase {
-  private static final Log LOG =
-    LogFactory.getLog(TestSetupWorkDir.class);
+  private static int NUM_SUB_DIRS = 3;
 
   /**
-   * Create a file in the given dir and set permissions r_xr_xr_x sothat no one
-   * can delete it directly(without doing chmod).
-   * Creates dir/subDir and dir/subDir/file
+   * Creates subdirectories under given dir and files under those subdirs.
+   * Creates dir/subDir1, dir/subDir1/file, dir/subDir2, dir/subDir2/file, etc.
    */
-  static void createFileAndSetPermissions(JobConf jobConf, Path dir)
+  static void createSubDirs(JobConf jobConf, Path dir)
        throws IOException {
-    Path subDir = new Path(dir, "subDir");
-    FileSystem fs = FileSystem.getLocal(jobConf);
-    fs.mkdirs(subDir);
-    Path p = new Path(subDir, "file");
-    DataOutputStream out = fs.create(p);
-    out.writeBytes("dummy input");
-    out.close();
-    // no write permission for subDir and subDir/file
-    try {
-      int ret = 0;
-      if((ret = FileUtil.chmod(subDir.toUri().getPath(), "a=rx", true)) != 0) {
-        LOG.warn("chmod failed for " + subDir + ";retVal=" + ret);
-      }
-    } catch(InterruptedException e) {
-      LOG.warn("Interrupted while doing chmod for " + subDir);
+    for (int i = 1; i <= NUM_SUB_DIRS; i++) {
+      Path subDir = new Path(dir, "subDir" + i);
+      FileSystem fs = FileSystem.getLocal(jobConf);
+      fs.mkdirs(subDir);
+      Path p = new Path(subDir, "file");
+      DataOutputStream out = fs.create(p);
+      out.writeBytes("dummy input");
+      out.close();
     }
   }
 
   /**
-   * Validates if setupWorkDir is properly cleaning up contents of workDir.
-   * TODO: other things of TaskRunner.setupWorkDir() related to distributed
-   * cache need to be validated.
+   * Validates if TaskRunner.deleteDirContents() is properly cleaning up the
+   * contents of workDir.
    */
   public void testSetupWorkDir() throws IOException {
     Path rootDir = new Path(System.getProperty("test.build.data",  "/tmp"),
@@ -76,9 +67,12 @@ public class TestSetupWorkDir extends TestCase {
       throw new IOException("Unable to create workDir " + myWorkDir);
     }
 
-    // create {myWorkDir}/subDir/file and set 555 perms for subDir and file
-    createFileAndSetPermissions(jConf, myWorkDir);
+    // create subDirs under work dir
+    createSubDirs(jConf, myWorkDir);
 
+    assertTrue("createDirAndSubDirs() did not create subdirs under "
+        + myWorkDir, fs.listStatus(myWorkDir).length == NUM_SUB_DIRS);
+    
     TaskRunner.deleteDirContents(jConf, new File(myWorkDir.toUri().getPath()));
     
     assertTrue("Contents of " + myWorkDir + " are not cleaned up properly.",