YARN-2035. FileSystemApplicationHistoryStore should not make working dir when it already exists. Contributed by Jonathan Eagles.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/d778abf0 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/d778abf0 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/d778abf0 Branch: refs/heads/HDFS-6584 Commit: d778abf022b415c64223153814d4188c2b3dd797 Parents: d16bfd1 Author: Zhijie Shen <[email protected]> Authored: Wed Aug 27 02:01:00 2014 -0700 Committer: Zhijie Shen <[email protected]> Committed: Wed Aug 27 02:01:00 2014 -0700 ---------------------------------------------------------------------- hadoop-yarn-project/CHANGES.txt | 3 + .../FileSystemApplicationHistoryStore.java | 14 ++++- .../TestFileSystemApplicationHistoryStore.java | 62 +++++++++++++++++++- 3 files changed, 75 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/d778abf0/hadoop-yarn-project/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index eefa547..36d304c 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -240,6 +240,9 @@ Release 2.6.0 - UNRELEASED YARN-2434. RM should not recover containers from previously failed attempt when AM restart is not enabled (Jian He via jlowe) + YARN-2035. FileSystemApplicationHistoryStore should not make working dir + when it already exists. (Jonathan Eagles via zjshen) + Release 2.5.1 - UNRELEASED INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/hadoop/blob/d778abf0/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/FileSystemApplicationHistoryStore.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/FileSystemApplicationHistoryStore.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/FileSystemApplicationHistoryStore.java index a5725eb..a2d9140 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/FileSystemApplicationHistoryStore.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/FileSystemApplicationHistoryStore.java @@ -110,15 +110,23 @@ public class FileSystemApplicationHistoryStore extends AbstractService super(FileSystemApplicationHistoryStore.class.getName()); } + protected FileSystem getFileSystem(Path path, Configuration conf) throws Exception { + return path.getFileSystem(conf); + } + @Override public void serviceInit(Configuration conf) throws Exception { Path fsWorkingPath = new Path(conf.get(YarnConfiguration.FS_APPLICATION_HISTORY_STORE_URI)); rootDirPath = new Path(fsWorkingPath, ROOT_DIR_NAME); try { - fs = fsWorkingPath.getFileSystem(conf); - fs.mkdirs(rootDirPath); - fs.setPermission(rootDirPath, ROOT_DIR_UMASK); + fs = getFileSystem(fsWorkingPath, conf); + + if (!fs.isDirectory(rootDirPath)) { + fs.mkdirs(rootDirPath); + fs.setPermission(rootDirPath, ROOT_DIR_UMASK); + } + } catch (IOException e) { LOG.error("Error when initializing FileSystemHistoryStorage", e); throw e; http://git-wip-us.apache.org/repos/asf/hadoop/blob/d778abf0/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestFileSystemApplicationHistoryStore.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestFileSystemApplicationHistoryStore.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestFileSystemApplicationHistoryStore.java index d31018c..552a5e5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestFileSystemApplicationHistoryStore.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestFileSystemApplicationHistoryStore.java @@ -20,9 +20,17 @@ package org.apache.hadoop.yarn.server.applicationhistoryservice; import java.io.IOException; import java.net.URI; +import java.net.URISyntaxException; import org.junit.Assert; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; @@ -53,6 +61,11 @@ public class TestFileSystemApplicationHistoryStore extends @Before public void setup() throws Exception { fs = new RawLocalFileSystem(); + initStore(fs); + } + + private void initStore(final FileSystem fs) throws IOException, + URISyntaxException { Configuration conf = new Configuration(); fs.initialize(new URI("/"), conf); fsWorkingPath = @@ -61,7 +74,12 @@ public class TestFileSystemApplicationHistoryStore extends fs.delete(fsWorkingPath, true); conf.set(YarnConfiguration.FS_APPLICATION_HISTORY_STORE_URI, fsWorkingPath.toString()); - store = new FileSystemApplicationHistoryStore(); + store = new FileSystemApplicationHistoryStore() { + @Override + protected FileSystem getFileSystem(Path path, Configuration conf) { + return fs; + } + }; store.init(conf); store.start(); } @@ -243,4 +261,46 @@ public class TestFileSystemApplicationHistoryStore extends testWriteHistoryData(3, false, true); testReadHistoryData(3, false, true); } + + @Test + public void testInitExistingWorkingDirectoryInSafeMode() throws Exception { + LOG.info("Starting testInitExistingWorkingDirectoryInSafeMode"); + tearDown(); + + // Setup file system to inject startup conditions + FileSystem fs = spy(new RawLocalFileSystem()); + doReturn(true).when(fs).isDirectory(any(Path.class)); + + try { + initStore(fs); + } catch (Exception e) { + Assert.fail("Exception should not be thrown: " + e); + } + + // Make sure that directory creation was not attempted + verify(fs, times(1)).isDirectory(any(Path.class)); + verify(fs, times(0)).mkdirs(any(Path.class)); + } + + @Test + public void testInitNonExistingWorkingDirectoryInSafeMode() throws Exception { + LOG.info("Starting testInitNonExistingWorkingDirectoryInSafeMode"); + tearDown(); + + // Setup file system to inject startup conditions + FileSystem fs = spy(new RawLocalFileSystem()); + doReturn(false).when(fs).isDirectory(any(Path.class)); + doThrow(new IOException()).when(fs).mkdirs(any(Path.class)); + + try { + initStore(fs); + Assert.fail("Exception should have been thrown"); + } catch (Exception e) { + // Expected failure + } + + // Make sure that directory creation was attempted + verify(fs, times(1)).isDirectory(any(Path.class)); + verify(fs, times(1)).mkdirs(any(Path.class)); + } }
