You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by vi...@apache.org on 2011/09/21 15:26:56 UTC
svn commit: r1173623 - in
/hadoop/common/trunk/hadoop-common-project/hadoop-common: CHANGES.txt
src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java
src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java
Author: vinodkv
Date: Wed Sep 21 13:26:55 2011
New Revision: 1173623
URL: http://svn.apache.org/viewvc?rev=1173623&view=rev
Log:
HADOOP-7575. Enhanced LocalDirAllocator to support fully-qualified paths. Contributed by Jonathan Eagles.
Modified:
hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1173623&r1=1173622&r2=1173623&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt Wed Sep 21 13:26:55 2011
@@ -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
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java?rev=1173623&r1=1173622&r2=1173623&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java Wed Sep 21 13:26:55 2011
@@ -264,9 +264,15 @@ public class LocalDirAllocator {
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);
}
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java?rev=1173623&r1=1173622&r2=1173623&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java Wed Sep 21 13:26:55 2011
@@ -20,40 +20,48 @@ package org.apache.hadoop.fs;
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 exten
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<Object[]> 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 class TestLocalDirAllocator exten
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();
+ }
+ }
}