You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mapreduce-commits@hadoop.apache.org by to...@apache.org on 2011/01/24 23:43:42 UTC

svn commit: r1063044 - in /hadoop/mapreduce/trunk: CHANGES.txt src/java/org/apache/hadoop/mapreduce/util/MRAsyncDiskService.java src/test/mapred/org/apache/hadoop/mapreduce/util/TestMRAsyncDiskService.java

Author: tomwhite
Date: Mon Jan 24 22:43:42 2011
New Revision: 1063044

URL: http://svn.apache.org/viewvc?rev=1063044&view=rev
Log:
MAPREDUCE-1382. MRAsyncDiscService should tolerate missing local.dir. Contributed by Zheng Shao and tomwhite.

Modified:
    hadoop/mapreduce/trunk/CHANGES.txt
    hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/MRAsyncDiskService.java
    hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestMRAsyncDiskService.java

Modified: hadoop/mapreduce/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/CHANGES.txt?rev=1063044&r1=1063043&r2=1063044&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/CHANGES.txt (original)
+++ hadoop/mapreduce/trunk/CHANGES.txt Mon Jan 24 22:43:42 2011
@@ -21,6 +21,9 @@ Trunk (unreleased changes)
     MAPREDUCE-1906. Lower minimum heartbeat interval for TaskTracker
     (Scott Carey and Todd Lipcon via todd)
 
+    MAPREDUCE-1382. MRAsyncDiscService should tolerate missing local.dir.
+    (Zheng Shao and tomwhite via tomwhite)
+
   OPTIMIZATIONS
 
   BUG FIXES

Modified: hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/MRAsyncDiskService.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/MRAsyncDiskService.java?rev=1063044&r1=1063043&r2=1063044&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/MRAsyncDiskService.java (original)
+++ hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/MRAsyncDiskService.java Mon Jan 24 22:43:42 2011
@@ -89,28 +89,32 @@ public class MRAsyncDiskService {
       // Create the root for file deletion
       Path absoluteSubdir = new Path(volumes[v], TOBEDELETED);
       if (!localFileSystem.mkdirs(absoluteSubdir)) {
-        throw new IOException("Cannot create " + TOBEDELETED + " in "
-            + volumes[v]);
+        // We should tolerate missing volumes. 
+        LOG.warn("Cannot create " + TOBEDELETED + " in " + volumes[v] + ". Ignored.");
       }
     }
     
     // Create tasks to delete the paths inside the volumes
     for (int v = 0 ; v < volumes.length; v++) {
       Path absoluteSubdir = new Path(volumes[v], TOBEDELETED);
-      // List all files inside the volumes
-      FileStatus[] files = localFileSystem.listStatus(absoluteSubdir);
-      for (int f = 0; f < files.length; f++) {
-        // Get the relative file name to the root of the volume
-        String absoluteFilename = files[f].getPath().toUri().getPath();
-        String relative = getRelativePathName(absoluteFilename, volumes[v]);
-        if (relative == null) {
-          // This should never happen
-          throw new IOException("Cannot delete " + absoluteFilename
-              + " because it's outside of " + volumes[v]);
+      FileStatus[] files = null;
+      try {
+        // List all files inside the volumes TOBEDELETED sub directory
+        files = localFileSystem.listStatus(absoluteSubdir);
+      } catch (Exception e) {
+        // Ignore exceptions in listStatus
+        // We tolerate missing sub directories.
+      }
+      if (files != null) {
+        for (int f = 0; f < files.length; f++) {
+          // Get the relative file name to the root of the volume
+          String absoluteFilename = files[f].getPath().toUri().getPath();
+          String relative = TOBEDELETED + Path.SEPARATOR_CHAR
+              + files[f].getPath().getName();
+          DeleteTask task = new DeleteTask(volumes[v], absoluteFilename,
+              relative);
+          execute(volumes[v], task);
         }
-        DeleteTask task = new DeleteTask(volumes[v], absoluteFilename,
-            relative);
-        execute(volumes[v], task);
       }
     }
   }
@@ -244,9 +248,15 @@ public class MRAsyncDiskService {
     newPathName = TOBEDELETED + Path.SEPARATOR_CHAR + newPathName;
     
     Path source = new Path(volume, pathName);
-    Path target = new Path(volume, newPathName); 
+    Path target = new Path(volume, newPathName);
     try {
       if (!localFileSystem.rename(source, target)) {
+        // If the source does not exists, return false.
+        // This is necessary because rename can return false if the source  
+        // does not exists.
+        if (!localFileSystem.exists(source)) {
+          return false;
+        }
         // Try to recreate the parent directory just in case it gets deleted.
         if (!localFileSystem.mkdirs(new Path(volume, TOBEDELETED))) {
           throw new IOException("Cannot create " + TOBEDELETED + " under "
@@ -296,19 +306,21 @@ public class MRAsyncDiskService {
   public void cleanupAllVolumes() throws IOException {
     for (int v = 0; v < volumes.length; v++) {
       // List all files inside the volumes
-      FileStatus[] files = localFileSystem.listStatus(new Path(volumes[v]));
-      for (int f = 0; f < files.length; f++) {
-        // Get the relative file name to the root of the volume
-        String absoluteFilename = files[f].getPath().toUri().getPath();
-        String relative = getRelativePathName(absoluteFilename, volumes[v]);
-        if (relative == null) {
-          // This should never happen
-          throw new IOException("Cannot delete " + absoluteFilename
-              + " because it's outside of " + volumes[v]);
-        }
-        // Do not delete the current TOBEDELETED
-        if (!TOBEDELETED.equals(relative)) {
-          moveAndDeleteRelativePath(volumes[v], relative);
+      FileStatus[] files = null;
+      try {
+        files = localFileSystem.listStatus(new Path(volumes[v]));
+      } catch (Exception e) {
+        // Ignore exceptions in listStatus
+        // We tolerate missing volumes.
+      }
+      if (files != null) {
+        for (int f = 0; f < files.length; f++) {
+          // Get the file name - the last component of the Path
+          String entryName = files[f].getPath().getName();
+          // Do not delete the current TOBEDELETED
+          if (!TOBEDELETED.equals(entryName)) {
+            moveAndDeleteRelativePath(volumes[v], entryName);
+          }
         }
       }
     }

Modified: hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestMRAsyncDiskService.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestMRAsyncDiskService.java?rev=1063044&r1=1063043&r2=1063044&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestMRAsyncDiskService.java (original)
+++ hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestMRAsyncDiskService.java Mon Jan 24 22:43:42 2011
@@ -28,6 +28,7 @@ import org.apache.commons.logging.LogFac
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.mapreduce.util.MRAsyncDiskService;
 import org.junit.Test;
 
@@ -40,6 +41,11 @@ public class TestMRAsyncDiskService exte
   
   private static String TEST_ROOT_DIR = new Path(System.getProperty(
       "test.build.data", "/tmp")).toString();
+  
+  @Override
+  protected void setUp() throws Exception {
+    FileUtil.fullyDelete(new File(TEST_ROOT_DIR));
+  }
 
   /**
    * Given 'pathname', compute an equivalent path relative to the cwd.
@@ -193,6 +199,8 @@ public class TestMRAsyncDiskService exte
     assertFalse(fb.exists());
     assertFalse(fc.exists());
     
+    assertFalse(service.moveAndDeleteRelativePath(vols[1], "not_exists"));
+    
     // asyncDiskService is NOT able to delete files outside all volumes.
     IOException ee = null;
     try {
@@ -275,10 +283,11 @@ public class TestMRAsyncDiskService exte
     String d = "d";
     
     // Create directories inside SUBDIR
-    File fa = new File(vols[0] + Path.SEPARATOR_CHAR + MRAsyncDiskService.TOBEDELETED, a);
-    File fb = new File(vols[1] + Path.SEPARATOR_CHAR + MRAsyncDiskService.TOBEDELETED, b);
-    File fc = new File(vols[1] + Path.SEPARATOR_CHAR + MRAsyncDiskService.TOBEDELETED, c);
-    File fd = new File(vols[1] + Path.SEPARATOR_CHAR + MRAsyncDiskService.TOBEDELETED, d);
+    String suffix = Path.SEPARATOR_CHAR + MRAsyncDiskService.TOBEDELETED;
+    File fa = new File(vols[0] + suffix, a);
+    File fb = new File(vols[1] + suffix, b);
+    File fc = new File(vols[1] + suffix, c);
+    File fd = new File(vols[1] + suffix, d);
     
     // Create the directories
     fa.mkdirs();
@@ -321,5 +330,20 @@ public class TestMRAsyncDiskService exte
           content.length);
     }
   }
+  
+  @Test
+  public void testToleratesSomeUnwritableVolumes() throws Throwable {
+    FileSystem localFileSystem = FileSystem.getLocal(new Configuration());
+    String[] vols = new String[]{TEST_ROOT_DIR + "/0",
+        TEST_ROOT_DIR + "/1"};
     
+    assertTrue(new File(vols[0]).mkdirs());
+    assertEquals(0, FileUtil.chmod(vols[0], "400")); // read only
+    try {
+      new MRAsyncDiskService(localFileSystem, vols);
+    } finally {
+      FileUtil.chmod(vols[0], "755"); // make writable again
+    }
+  }
+  
 }