From d50ecc38a3e0f5405bcf14b22934e7a89a190f6e Mon Sep 17 00:00:00 2001 From: Vinod Kumar Vavilapalli Date: Wed, 21 Sep 2011 13:26:55 +0000 Subject: [PATCH] HADOOP-7575. Enhanced LocalDirAllocator to support fully-qualified paths. Contributed by Jonathan Eagles. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1173623 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 + .../apache/hadoop/fs/LocalDirAllocator.java | 12 +- .../hadoop/fs/TestLocalDirAllocator.java | 221 ++++++++++++------ 3 files changed, 159 insertions(+), 77 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index a20b02ed23..fe9710f5cd 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -392,6 +392,9 @@ Release 0.23.0 - Unreleased so that servers like Yarn WebApp can get filtered the paths served by their own injected servlets. (Thomas Graves via vinodkv) + HADOOP-7575. Enhanced LocalDirAllocator to support fully-qualified + paths. (Jonathan Eagles via vinodkv) + OPTIMIZATIONS HADOOP-7333. Performance improvement in PureJavaCrc32. (Eric Caspole diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java index 71c8235757..d1eae086f9 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java @@ -264,9 +264,15 @@ private synchronized void confChanged(Configuration conf) Path tmpDir = new Path(localDirs[i]); if(localFS.mkdirs(tmpDir)|| localFS.exists(tmpDir)) { try { - DiskChecker.checkDir(new File(localDirs[i])); - dirs.add(localDirs[i]); - dfList.add(new DF(new File(localDirs[i]), 30000)); + + File tmpFile = tmpDir.isAbsolute() + ? new File(localFS.makeQualified(tmpDir).toUri()) + : new File(localDirs[i]); + + DiskChecker.checkDir(tmpFile); + dirs.add(tmpFile.getPath()); + dfList.add(new DF(tmpFile, 30000)); + } catch (DiskErrorException de) { LOG.warn( localDirs[i] + " is not writable\n", de); } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java index 1e22a73bba..e87f2d122b 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java @@ -20,40 +20,48 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.util.Shell; -import junit.framework.TestCase; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import org.junit.Test; + +import static org.junit.Assert.*; /** This test LocalDirAllocator works correctly; - * Every test case uses different buffer dirs to + * Every test case uses different buffer dirs to * enforce the AllocatorPerContext initialization. * This test does not run on Cygwin because under Cygwin * a directory can be created in a read-only directory * which breaks this test. - */ -public class TestLocalDirAllocator extends TestCase { + */ +@RunWith(Parameterized.class) +public class TestLocalDirAllocator { final static private Configuration conf = new Configuration(); final static private String BUFFER_DIR_ROOT = "build/test/temp"; + final static private String ABSOLUTE_DIR_ROOT; + final static private String QUALIFIED_DIR_ROOT; final static private Path BUFFER_PATH_ROOT = new Path(BUFFER_DIR_ROOT); final static private File BUFFER_ROOT = new File(BUFFER_DIR_ROOT); - final static private String BUFFER_DIR[] = new String[] { - BUFFER_DIR_ROOT+"/tmp0", BUFFER_DIR_ROOT+"/tmp1", BUFFER_DIR_ROOT+"/tmp2", - BUFFER_DIR_ROOT+"/tmp3", BUFFER_DIR_ROOT+"/tmp4", BUFFER_DIR_ROOT+"/tmp5", - BUFFER_DIR_ROOT+"/tmp6"}; - final static private Path BUFFER_PATH[] = new Path[] { - new Path(BUFFER_DIR[0]), new Path(BUFFER_DIR[1]), new Path(BUFFER_DIR[2]), - new Path(BUFFER_DIR[3]), new Path(BUFFER_DIR[4]), new Path(BUFFER_DIR[5]), - new Path(BUFFER_DIR[6])}; - final static private String CONTEXT = "dfs.client.buffer.dir"; + final static private String CONTEXT = "fs.client.buffer.dir"; final static private String FILENAME = "block"; - final static private LocalDirAllocator dirAllocator = + final static private LocalDirAllocator dirAllocator = new LocalDirAllocator(CONTEXT); static LocalFileSystem localFs; final static private boolean isWindows = System.getProperty("os.name").startsWith("Windows"); final static int SMALL_FILE_SIZE = 100; + final static private String RELATIVE = "/RELATIVE"; + final static private String ABSOLUTE = "/ABSOLUTE"; + final static private String QUALIFIED = "/QUALIFIED"; + final private String ROOT; + final private String PREFIX; + static { try { localFs = FileSystem.getLocal(conf); @@ -63,170 +71,214 @@ public class TestLocalDirAllocator extends TestCase { e.printStackTrace(); System.exit(-1); } + + ABSOLUTE_DIR_ROOT = new Path(localFs.getWorkingDirectory(), + BUFFER_DIR_ROOT).toUri().getPath(); + QUALIFIED_DIR_ROOT = new Path(localFs.getWorkingDirectory(), + BUFFER_DIR_ROOT).toUri().toString(); + } + + public TestLocalDirAllocator(String root, String prefix) { + ROOT = root; + PREFIX = prefix; + } + + @Parameters + public static Collection params() { + Object [][] data = new Object[][] { + { BUFFER_DIR_ROOT, RELATIVE }, + { ABSOLUTE_DIR_ROOT, ABSOLUTE }, + { QUALIFIED_DIR_ROOT, QUALIFIED } + }; + + return Arrays.asList(data); } private static void rmBufferDirs() throws IOException { assertTrue(!localFs.exists(BUFFER_PATH_ROOT) || localFs.delete(BUFFER_PATH_ROOT, true)); } - - private void validateTempDirCreation(int i) throws IOException { + + private static void validateTempDirCreation(String dir) throws IOException { File result = createTempFile(SMALL_FILE_SIZE); - assertTrue("Checking for " + BUFFER_DIR[i] + " in " + result + " - FAILED!", - result.getPath().startsWith(new File(BUFFER_DIR[i], FILENAME).getPath())); + assertTrue("Checking for " + dir + " in " + result + " - FAILED!", + result.getPath().startsWith(new Path(dir, FILENAME).toUri().getPath())); } - - private File createTempFile() throws IOException { - File result = dirAllocator.createTmpFileForWrite(FILENAME, -1, conf); - result.delete(); - return result; + + private static File createTempFile() throws IOException { + return createTempFile(-1); } - - private File createTempFile(long size) throws IOException { + + private static File createTempFile(long size) throws IOException { File result = dirAllocator.createTmpFileForWrite(FILENAME, size, conf); result.delete(); return result; } - - /** Two buffer dirs. The first dir does not exist & is on a read-only disk; + + private String buildBufferDir(String dir, int i) { + return dir + PREFIX + i; + } + + /** Two buffer dirs. The first dir does not exist & is on a read-only disk; * The second dir exists & is RW * @throws Exception */ + @Test public void test0() throws Exception { if (isWindows) return; + String dir0 = buildBufferDir(ROOT, 0); + String dir1 = buildBufferDir(ROOT, 1); try { - conf.set(CONTEXT, BUFFER_DIR[0]+","+BUFFER_DIR[1]); - assertTrue(localFs.mkdirs(BUFFER_PATH[1])); + conf.set(CONTEXT, dir0 + "," + dir1); + assertTrue(localFs.mkdirs(new Path(dir1))); BUFFER_ROOT.setReadOnly(); - validateTempDirCreation(1); - validateTempDirCreation(1); + validateTempDirCreation(dir1); + validateTempDirCreation(dir1); } finally { Shell.execCommand(new String[]{"chmod", "u+w", BUFFER_DIR_ROOT}); rmBufferDirs(); } } - - /** Two buffer dirs. The first dir exists & is on a read-only disk; + + /** Two buffer dirs. The first dir exists & is on a read-only disk; * The second dir exists & is RW * @throws Exception */ + @Test public void test1() throws Exception { if (isWindows) return; + String dir1 = buildBufferDir(ROOT, 1); + String dir2 = buildBufferDir(ROOT, 2); try { - conf.set(CONTEXT, BUFFER_DIR[1]+","+BUFFER_DIR[2]); - assertTrue(localFs.mkdirs(BUFFER_PATH[2])); + conf.set(CONTEXT, dir1 + "," + dir2); + assertTrue(localFs.mkdirs(new Path(dir2))); BUFFER_ROOT.setReadOnly(); - validateTempDirCreation(2); - validateTempDirCreation(2); + validateTempDirCreation(dir2); + validateTempDirCreation(dir2); } finally { Shell.execCommand(new String[]{"chmod", "u+w", BUFFER_DIR_ROOT}); rmBufferDirs(); } } /** Two buffer dirs. Both do not exist but on a RW disk. - * Check if tmp dirs are allocated in a round-robin + * Check if tmp dirs are allocated in a round-robin */ + @Test public void test2() throws Exception { if (isWindows) return; + String dir2 = buildBufferDir(ROOT, 2); + String dir3 = buildBufferDir(ROOT, 3); try { - conf.set(CONTEXT, BUFFER_DIR[2]+","+BUFFER_DIR[3]); + conf.set(CONTEXT, dir2 + "," + dir3); // create the first file, and then figure the round-robin sequence createTempFile(SMALL_FILE_SIZE); int firstDirIdx = (dirAllocator.getCurrentDirectoryIndex() == 0) ? 2 : 3; int secondDirIdx = (firstDirIdx == 2) ? 3 : 2; - + // check if tmp dirs are allocated in a round-robin manner - validateTempDirCreation(firstDirIdx); - validateTempDirCreation(secondDirIdx); - validateTempDirCreation(firstDirIdx); + validateTempDirCreation(buildBufferDir(ROOT, firstDirIdx)); + validateTempDirCreation(buildBufferDir(ROOT, secondDirIdx)); + validateTempDirCreation(buildBufferDir(ROOT, firstDirIdx)); } finally { rmBufferDirs(); } } - /** Two buffer dirs. Both exists and on a R/W disk. + /** Two buffer dirs. Both exists and on a R/W disk. * Later disk1 becomes read-only. * @throws Exception */ + @Test public void test3() throws Exception { if (isWindows) return; + String dir3 = buildBufferDir(ROOT, 3); + String dir4 = buildBufferDir(ROOT, 4); try { - conf.set(CONTEXT, BUFFER_DIR[3]+","+BUFFER_DIR[4]); - assertTrue(localFs.mkdirs(BUFFER_PATH[3])); - assertTrue(localFs.mkdirs(BUFFER_PATH[4])); - - // create the first file with size, and then figure the round-robin sequence + conf.set(CONTEXT, dir3 + "," + dir4); + assertTrue(localFs.mkdirs(new Path(dir3))); + assertTrue(localFs.mkdirs(new Path(dir4))); + + // Create the first small file createTempFile(SMALL_FILE_SIZE); + // Determine the round-robin sequence int nextDirIdx = (dirAllocator.getCurrentDirectoryIndex() == 0) ? 3 : 4; - validateTempDirCreation(nextDirIdx); + validateTempDirCreation(buildBufferDir(ROOT, nextDirIdx)); // change buffer directory 2 to be read only - new File(BUFFER_DIR[4]).setReadOnly(); - validateTempDirCreation(3); - validateTempDirCreation(3); + new File(new Path(dir4).toUri().getPath()).setReadOnly(); + validateTempDirCreation(dir3); + validateTempDirCreation(dir3); } finally { rmBufferDirs(); } } - + /** * Two buffer dirs, on read-write disk. - * + * * Try to create a whole bunch of files. * Verify that they do indeed all get created where they should. - * + * * Would ideally check statistical properties of distribution, but * we don't have the nerve to risk false-positives here. - * + * * @throws Exception */ static final int TRIALS = 100; + @Test public void test4() throws Exception { if (isWindows) return; + String dir5 = buildBufferDir(ROOT, 5); + String dir6 = buildBufferDir(ROOT, 6); try { - conf.set(CONTEXT, BUFFER_DIR[5]+","+BUFFER_DIR[6]); - assertTrue(localFs.mkdirs(BUFFER_PATH[5])); - assertTrue(localFs.mkdirs(BUFFER_PATH[6])); - + conf.set(CONTEXT, dir5 + "," + dir6); + assertTrue(localFs.mkdirs(new Path(dir5))); + assertTrue(localFs.mkdirs(new Path(dir6))); + int inDir5=0, inDir6=0; for(int i = 0; i < TRIALS; ++i) { File result = createTempFile(); - if(result.getPath().startsWith(new File(BUFFER_DIR[5], FILENAME).getPath())) { + if(result.getPath().startsWith( + new Path(dir5, FILENAME).toUri().getPath())) { inDir5++; - } else if(result.getPath().startsWith(new File(BUFFER_DIR[6], FILENAME).getPath())) { + } else if(result.getPath().startsWith( + new Path(dir6, FILENAME).toUri().getPath())) { inDir6++; } result.delete(); } - - assertTrue( inDir5 + inDir6 == TRIALS); - + + assertTrue(inDir5 + inDir6 == TRIALS); + } finally { rmBufferDirs(); } } - - /** Two buffer dirs. The first dir does not exist & is on a read-only disk; + + /** Two buffer dirs. The first dir does not exist & is on a read-only disk; * The second dir exists & is RW * getLocalPathForWrite with checkAccess set to false should create a parent * directory. With checkAccess true, the directory should not be created. * @throws Exception */ + @Test public void testLocalPathForWriteDirCreation() throws IOException { + String dir0 = buildBufferDir(ROOT, 0); + String dir1 = buildBufferDir(ROOT, 1); try { - conf.set(CONTEXT, BUFFER_DIR[0] + "," + BUFFER_DIR[1]); - assertTrue(localFs.mkdirs(BUFFER_PATH[1])); + conf.set(CONTEXT, dir0 + "," + dir1); + assertTrue(localFs.mkdirs(new Path(dir1))); BUFFER_ROOT.setReadOnly(); Path p1 = - dirAllocator.getLocalPathForWrite("p1/x", SMALL_FILE_SIZE, conf); + dirAllocator.getLocalPathForWrite("p1/x", SMALL_FILE_SIZE, conf); assertTrue(localFs.getFileStatus(p1.getParent()).isDirectory()); Path p2 = - dirAllocator.getLocalPathForWrite("p2/x", SMALL_FILE_SIZE, conf, - false); + dirAllocator.getLocalPathForWrite("p2/x", SMALL_FILE_SIZE, conf, + false); try { localFs.getFileStatus(p2.getParent()); } catch (Exception e) { @@ -237,5 +289,26 @@ public void testLocalPathForWriteDirCreation() throws IOException { rmBufferDirs(); } } - + + /** Test no side effect files are left over. After creating a temp + * temp file, remove both the temp file and its parent. Verify that + * no files or directories are left over as can happen when File objects + * are mistakenly created from fully qualified path strings. + * @throws IOException + */ + @Test + public void testNoSideEffects() throws IOException { + if (isWindows) return; + String dir = buildBufferDir(ROOT, 0); + try { + conf.set(CONTEXT, dir); + File result = dirAllocator.createTmpFileForWrite(FILENAME, -1, conf); + assertTrue(result.delete()); + assertTrue(result.getParentFile().delete()); + assertFalse(new File(dir).exists()); + } finally { + Shell.execCommand(new String[]{"chmod", "u+w", BUFFER_DIR_ROOT}); + rmBufferDirs(); + } + } }