From 1425c65b5e2cc7f57bf0ac51e4b6bb546736b601 Mon Sep 17 00:00:00 2001 From: Robert Joseph Evans Date: Fri, 17 Feb 2012 16:27:27 +0000 Subject: [PATCH] HADOOP-8054. NPE with FilterFileSystem (Daryn Sharp via bobby) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1245637 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 1 + .../apache/hadoop/fs/FilterFileSystem.java | 5 + .../org/apache/hadoop/fs/LocalFileSystem.java | 7 - .../hadoop/fs/TestFilterFileSystem.java | 125 +++++++++++++++++- 4 files changed, 129 insertions(+), 9 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 2e9ba58536..a199c6e1b8 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -174,6 +174,7 @@ Release 0.23.2 - UNRELEASED (sharad, todd via todd) BUG FIXES + HADOOP-8054 NPE with FilterFileSystem (Daryn Sharp via bobby) HADOOP-8042 When copying a file out of HDFS, modifying it, and uploading it back into HDFS, the put fails due to a CRC mismatch diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java index 9a152cb6d7..91ee2ae710 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java @@ -80,6 +80,11 @@ public FileSystem getRawFileSystem() { */ public void initialize(URI name, Configuration conf) throws IOException { super.initialize(name, conf); + // this is less than ideal, but existing filesystems sometimes neglect + // to initialize the embedded filesystem + if (fs.getConf() == null) { + fs.initialize(name, conf); + } String scheme = name.getScheme(); if (!scheme.equals(fs.getUri().getScheme())) { swapScheme = scheme; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalFileSystem.java index 88ee7b8224..4aff81114b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalFileSystem.java @@ -48,13 +48,6 @@ public LocalFileSystem(FileSystem rawLocalFileSystem) { super(rawLocalFileSystem); } - @Override - public void initialize(URI uri, Configuration conf) throws IOException { - super.initialize(uri, conf); - // ctor didn't initialize the filtered fs - getRawFileSystem().initialize(uri, conf); - } - /** Convert a path to a File. */ public File pathToFile(Path path) { return ((RawLocalFileSystem)fs).pathToFile(path); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java index 1ea721aa0d..364f46d2a4 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java @@ -18,24 +18,39 @@ package org.apache.hadoop.fs; +import static org.junit.Assert.*; +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + import java.io.IOException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.net.URI; import java.util.EnumSet; import java.util.Iterator; -import junit.framework.TestCase; import org.apache.commons.logging.Log; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.fs.Options.CreateOpts; import org.apache.hadoop.fs.Options.Rename; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.util.Progressable; +import org.junit.BeforeClass; +import org.junit.Test; -public class TestFilterFileSystem extends TestCase { +public class TestFilterFileSystem { private static final Log LOG = FileSystem.LOG; + private static final Configuration conf = new Configuration(); + @BeforeClass + public static void setup() { + conf.set("fs.flfs.impl", FilterLocalFileSystem.class.getName()); + conf.setBoolean("fs.flfs.impl.disable.cache", true); + conf.setBoolean("fs.file.impl.disable.cache", true); + } + public static class DontCheck { public BlockLocation[] getFileBlockLocations(Path p, long start, long len) { return null; } @@ -153,6 +168,7 @@ public Token getDelegationToken(String renewer) throws IOException { } + @Test public void testFilterFileSystem() throws Exception { for (Method m : FileSystem.class.getDeclaredMethods()) { if (Modifier.isStatic(m.getModifiers())) @@ -176,4 +192,109 @@ public void testFilterFileSystem() throws Exception { } } + @Test + public void testFilterEmbedInit() throws Exception { + FileSystem mockFs = createMockFs(false); // no conf = need init + checkInit(new FilterFileSystem(mockFs), true); + } + + @Test + public void testFilterEmbedNoInit() throws Exception { + FileSystem mockFs = createMockFs(true); // has conf = skip init + checkInit(new FilterFileSystem(mockFs), false); + } + + @Test + public void testLocalEmbedInit() throws Exception { + FileSystem mockFs = createMockFs(false); // no conf = need init + checkInit(new LocalFileSystem(mockFs), true); + } + + @Test + public void testLocalEmbedNoInit() throws Exception { + FileSystem mockFs = createMockFs(true); // has conf = skip init + checkInit(new LocalFileSystem(mockFs), false); + } + + private FileSystem createMockFs(boolean useConf) { + FileSystem mockFs = mock(FileSystem.class); + when(mockFs.getUri()).thenReturn(URI.create("mock:/")); + when(mockFs.getConf()).thenReturn(useConf ? conf : null); + return mockFs; + } + + @Test + public void testGetLocalFsSetsConfs() throws Exception { + LocalFileSystem lfs = FileSystem.getLocal(conf); + checkFsConf(lfs, conf, 2); + } + + @Test + public void testGetFilterLocalFsSetsConfs() throws Exception { + FilterFileSystem flfs = + (FilterFileSystem) FileSystem.get(URI.create("flfs:/"), conf); + checkFsConf(flfs, conf, 3); + } + + @Test + public void testInitLocalFsSetsConfs() throws Exception { + LocalFileSystem lfs = new LocalFileSystem(); + checkFsConf(lfs, null, 2); + lfs.initialize(lfs.getUri(), conf); + checkFsConf(lfs, conf, 2); + } + + @Test + public void testInitFilterFsSetsEmbedConf() throws Exception { + LocalFileSystem lfs = new LocalFileSystem(); + checkFsConf(lfs, null, 2); + FilterFileSystem ffs = new FilterFileSystem(lfs); + assertEquals(lfs, ffs.getRawFileSystem()); + checkFsConf(ffs, null, 3); + ffs.initialize(URI.create("filter:/"), conf); + checkFsConf(ffs, conf, 3); + } + + @Test + public void testInitFilterLocalFsSetsEmbedConf() throws Exception { + FilterFileSystem flfs = new FilterLocalFileSystem(); + assertEquals(LocalFileSystem.class, flfs.getRawFileSystem().getClass()); + checkFsConf(flfs, null, 3); + flfs.initialize(URI.create("flfs:/"), conf); + checkFsConf(flfs, conf, 3); + } + + private void checkInit(FilterFileSystem fs, boolean expectInit) + throws Exception { + URI uri = URI.create("filter:/"); + fs.initialize(uri, conf); + + FileSystem embedFs = fs.getRawFileSystem(); + if (expectInit) { + verify(embedFs, times(1)).initialize(eq(uri), eq(conf)); + } else { + verify(embedFs, times(0)).initialize(any(URI.class), any(Configuration.class)); + } + } + + // check the given fs's conf, and all its filtered filesystems + private void checkFsConf(FileSystem fs, Configuration conf, int expectDepth) { + int depth = 0; + while (true) { + depth++; + assertFalse("depth "+depth+">"+expectDepth, depth > expectDepth); + assertEquals(conf, fs.getConf()); + if (!(fs instanceof FilterFileSystem)) { + break; + } + fs = ((FilterFileSystem) fs).getRawFileSystem(); + } + assertEquals(expectDepth, depth); + } + + private static class FilterLocalFileSystem extends FilterFileSystem { + FilterLocalFileSystem() { + super(new LocalFileSystem()); + } + } }