From 1dd03cc4b573270dc960117c3b6c74bb78215caa Mon Sep 17 00:00:00 2001 From: Abhishek Das Date: Tue, 13 Jul 2021 12:47:43 -0700 Subject: [PATCH] HADOOP-17028. ViewFS should initialize mounted target filesystems lazily. Contributed by Abhishek Das (#2260) --- .../apache/hadoop/fs/viewfs/InodeTree.java | 79 +++++++++----- .../hadoop/fs/viewfs/ViewFileSystem.java | 101 +++++++++++++----- .../viewfs/ViewFileSystemOverloadScheme.java | 11 +- .../org/apache/hadoop/fs/viewfs/ViewFs.java | 38 +++++-- .../hadoop/fs/viewfs/TestRegexMountPoint.java | 12 ++- .../hadoop/fs/viewfs/TestViewFsConfig.java | 3 +- .../fs/viewfs/ViewFileSystemBaseTest.java | 44 ++++++++ .../fs/viewfs/TestViewFileSystemHdfs.java | 73 +++++++++++++ ...ileSystemOverloadSchemeWithHdfsScheme.java | 6 +- .../hadoop/fs/viewfs/TestViewFsHdfs.java | 78 ++++++++++++++ ...wFileSystemOverloadSchemeWithDFSAdmin.java | 6 +- 11 files changed, 379 insertions(+), 72 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java index 79c323aa35..1b9cf675d1 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.fs.viewfs; +import java.util.function.Function; import org.apache.hadoop.thirdparty.com.google.common.base.Preconditions; import java.io.FileNotFoundException; import java.io.IOException; @@ -257,7 +258,10 @@ enum LinkType { */ static class INodeLink extends INode { final URI[] targetDirLinkList; - final T targetFileSystem; // file system object created from the link. + private T targetFileSystem; // file system object created from the link. + // Function to initialize file system. Only applicable for simple links + private Function fileSystemInitMethod; + private final Object lock = new Object(); /** * Construct a mergeLink or nfly. @@ -273,11 +277,13 @@ static class INodeLink extends INode { * Construct a simple link (i.e. not a mergeLink). */ INodeLink(final String pathToNode, final UserGroupInformation aUgi, - final T targetFs, final URI aTargetDirLink) { + Function createFileSystemMethod, + final URI aTargetDirLink) { super(pathToNode, aUgi); - targetFileSystem = targetFs; + targetFileSystem = null; targetDirLinkList = new URI[1]; targetDirLinkList[0] = aTargetDirLink; + this.fileSystemInitMethod = createFileSystemMethod; } /** @@ -298,7 +304,30 @@ boolean isInternalDir() { return false; } - public T getTargetFileSystem() { + /** + * Get the instance of FileSystem to use, creating one if needed. + * @return An Initialized instance of T + * @throws IOException + */ + public T getTargetFileSystem() throws IOException { + if (targetFileSystem != null) { + return targetFileSystem; + } + // For non NFLY and MERGE links, we initialize the FileSystem when the + // corresponding mount path is accessed. + if (targetDirLinkList.length == 1) { + synchronized (lock) { + if (targetFileSystem != null) { + return targetFileSystem; + } + targetFileSystem = fileSystemInitMethod.apply(targetDirLinkList[0]); + if (targetFileSystem == null) { + throw new IOException( + "Could not initialize target File System for URI : " + + targetDirLinkList[0]); + } + } + } return targetFileSystem; } } @@ -359,7 +388,7 @@ private void createLink(final String src, final String target, switch (linkType) { case SINGLE: newLink = new INodeLink(fullPath, aUgi, - getTargetFileSystem(new URI(target)), new URI(target)); + initAndGetTargetFs(), new URI(target)); break; case SINGLE_FALLBACK: case MERGE_SLASH: @@ -385,8 +414,7 @@ private void createLink(final String src, final String target, * 3 abstract methods. * @throws IOException */ - protected abstract T getTargetFileSystem(URI uri) - throws UnsupportedFileSystemException, URISyntaxException, IOException; + protected abstract Function initAndGetTargetFs(); protected abstract T getTargetFileSystem(INodeDir dir) throws URISyntaxException, IOException; @@ -589,7 +617,7 @@ protected InodeTree(final Configuration config, final String viewName, if (isMergeSlashConfigured) { Preconditions.checkNotNull(mergeSlashTarget); root = new INodeLink(mountTableName, ugi, - getTargetFileSystem(new URI(mergeSlashTarget)), + initAndGetTargetFs(), new URI(mergeSlashTarget)); mountPoints.add(new MountPoint("/", (INodeLink) root)); rootFallbackLink = null; @@ -608,8 +636,7 @@ protected InodeTree(final Configuration config, final String viewName, + "not allowed."); } fallbackLink = new INodeLink(mountTableName, ugi, - getTargetFileSystem(new URI(le.getTarget())), - new URI(le.getTarget())); + initAndGetTargetFs(), new URI(le.getTarget())); continue; case REGEX: addRegexMountEntry(le); @@ -633,9 +660,8 @@ protected InodeTree(final Configuration config, final String viewName, FileSystem.LOG .info("Empty mount table detected for {} and considering itself " + "as a linkFallback.", theUri); - rootFallbackLink = - new INodeLink(mountTableName, ugi, getTargetFileSystem(theUri), - theUri); + rootFallbackLink = new INodeLink(mountTableName, ugi, + initAndGetTargetFs(), theUri); getRootDir().addFallbackLink(rootFallbackLink); } } @@ -733,10 +759,10 @@ boolean isLastInternalDirLink() { * @param p - input path * @param resolveLastComponent * @return ResolveResult which allows further resolution of the remaining path - * @throws FileNotFoundException + * @throws IOException */ ResolveResult resolve(final String p, final boolean resolveLastComponent) - throws FileNotFoundException { + throws IOException { ResolveResult resolveResult = null; String[] path = breakIntoPathComponents(p); if (path.length <= 1) { // special case for when path is "/" @@ -880,19 +906,20 @@ protected ResolveResult buildResolveResultForRegexMountPoint( ResultKind resultKind, String resolvedPathStr, String targetOfResolvedPathStr, Path remainingPath) { try { - T targetFs = getTargetFileSystem( - new URI(targetOfResolvedPathStr)); + T targetFs = initAndGetTargetFs() + .apply(new URI(targetOfResolvedPathStr)); + if (targetFs == null) { + LOGGER.error(String.format( + "Not able to initialize target file system." + + " ResultKind:%s, resolvedPathStr:%s," + + " targetOfResolvedPathStr:%s, remainingPath:%s," + + " will return null.", + resultKind, resolvedPathStr, targetOfResolvedPathStr, + remainingPath)); + return null; + } return new ResolveResult(resultKind, targetFs, resolvedPathStr, remainingPath, true); - } catch (IOException ex) { - LOGGER.error(String.format( - "Got Exception while build resolve result." - + " ResultKind:%s, resolvedPathStr:%s," - + " targetOfResolvedPathStr:%s, remainingPath:%s," - + " will return null.", - resultKind, resolvedPathStr, targetOfResolvedPathStr, remainingPath), - ex); - return null; } catch (URISyntaxException uex) { LOGGER.error(String.format( "Got Exception while build resolve result." diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 708d361c28..ceb72437a6 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -26,10 +26,12 @@ import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; +import java.util.function.Function; import java.io.FileNotFoundException; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -302,7 +304,7 @@ public void initialize(final URI theUri, final Configuration conf) enableInnerCache = config.getBoolean(CONFIG_VIEWFS_ENABLE_INNER_CACHE, CONFIG_VIEWFS_ENABLE_INNER_CACHE_DEFAULT); FsGetter fsGetter = fsGetter(); - final InnerCache innerCache = new InnerCache(fsGetter); + cache = new InnerCache(fsGetter); // Now build client side view (i.e. client side mount table) from config. final String authority = theUri.getAuthority(); String tableName = authority; @@ -318,15 +320,32 @@ public void initialize(final URI theUri, final Configuration conf) fsState = new InodeTree(conf, tableName, myUri, initingUriAsFallbackOnNoMounts) { @Override - protected FileSystem getTargetFileSystem(final URI uri) - throws URISyntaxException, IOException { - FileSystem fs; - if (enableInnerCache) { - fs = innerCache.get(uri, config); - } else { - fs = fsGetter.get(uri, config); - } - return new ChRootedFileSystem(fs, uri); + protected Function initAndGetTargetFs() { + return new Function() { + @Override + public FileSystem apply(final URI uri) { + FileSystem fs; + try { + fs = ugi.doAs(new PrivilegedExceptionAction() { + @Override + public FileSystem run() throws IOException { + if (enableInnerCache) { + synchronized (cache) { + return cache.get(uri, config); + } + } else { + return fsGetter().get(uri, config); + } + } + }); + return new ChRootedFileSystem(fs, uri); + } catch (IOException | InterruptedException ex) { + LOG.error("Could not initialize the underlying FileSystem " + + "object. Exception: " + ex.toString()); + } + return null; + } + }; } @Override @@ -350,13 +369,6 @@ protected FileSystem getTargetFileSystem(final String settings, } catch (URISyntaxException e) { throw new IOException("URISyntax exception: " + theUri); } - - if (enableInnerCache) { - // All fs instances are created and cached on startup. The cache is - // readonly after the initialize() so the concurrent access of the cache - // is safe. - cache = innerCache; - } } /** @@ -388,7 +400,7 @@ public URI getUri() { @Override public Path resolvePath(final Path f) throws IOException { final InodeTree.ResolveResult res; - res = fsState.resolve(getUriPath(f), true); + res = fsState.resolve(getUriPath(f), true); if (res.isInternalDir()) { return f; } @@ -908,10 +920,35 @@ public void removeXAttr(Path path, String name) throws IOException { public void setVerifyChecksum(final boolean verifyChecksum) { List> mountPoints = fsState.getMountPoints(); + Map fsMap = initializeMountedFileSystems(mountPoints); for (InodeTree.MountPoint mount : mountPoints) { - mount.target.targetFileSystem.setVerifyChecksum(verifyChecksum); + fsMap.get(mount.src).setVerifyChecksum(verifyChecksum); } } + + /** + * Initialize the target filesystem for all mount points. + * @param mountPoints The mount points + * @return Mapping of mount point and the initialized target filesystems + * @throws RuntimeException when the target file system cannot be initialized + */ + private Map initializeMountedFileSystems( + List> mountPoints) { + FileSystem fs = null; + Map fsMap = new HashMap<>(mountPoints.size()); + for (InodeTree.MountPoint mount : mountPoints) { + try { + fs = mount.target.getTargetFileSystem(); + fsMap.put(mount.src, fs); + } catch (IOException ex) { + String errMsg = "Not able to initialize FileSystem for mount path " + + mount.src + " with exception " + ex; + LOG.error(errMsg); + throw new RuntimeException(errMsg, ex); + } + } + return fsMap; + } @Override public long getDefaultBlockSize() { @@ -936,6 +973,9 @@ public long getDefaultBlockSize(Path f) { return res.targetFileSystem.getDefaultBlockSize(res.remainingPath); } catch (FileNotFoundException e) { throw new NotInMountpointException(f, "getDefaultBlockSize"); + } catch (IOException e) { + throw new RuntimeException("Not able to initialize fs in " + + " getDefaultBlockSize for path " + f + " with exception", e); } } @@ -947,6 +987,9 @@ public short getDefaultReplication(Path f) { return res.targetFileSystem.getDefaultReplication(res.remainingPath); } catch (FileNotFoundException e) { throw new NotInMountpointException(f, "getDefaultReplication"); + } catch (IOException e) { + throw new RuntimeException("Not able to initialize fs in " + + " getDefaultReplication for path " + f + " with exception", e); } } @@ -979,8 +1022,9 @@ public QuotaUsage getQuotaUsage(Path f) throws IOException { public void setWriteChecksum(final boolean writeChecksum) { List> mountPoints = fsState.getMountPoints(); + Map fsMap = initializeMountedFileSystems(mountPoints); for (InodeTree.MountPoint mount : mountPoints) { - mount.target.targetFileSystem.setWriteChecksum(writeChecksum); + fsMap.get(mount.src).setWriteChecksum(writeChecksum); } } @@ -988,16 +1032,23 @@ public void setWriteChecksum(final boolean writeChecksum) { public FileSystem[] getChildFileSystems() { List> mountPoints = fsState.getMountPoints(); + Map fsMap = initializeMountedFileSystems(mountPoints); Set children = new HashSet(); for (InodeTree.MountPoint mountPoint : mountPoints) { - FileSystem targetFs = mountPoint.target.targetFileSystem; + FileSystem targetFs = fsMap.get(mountPoint.src); children.addAll(Arrays.asList(targetFs.getChildFileSystems())); } - if (fsState.isRootInternalDir() && fsState.getRootFallbackLink() != null) { - children.addAll(Arrays.asList( - fsState.getRootFallbackLink().targetFileSystem - .getChildFileSystems())); + try { + if (fsState.isRootInternalDir() && + fsState.getRootFallbackLink() != null) { + children.addAll(Arrays.asList( + fsState.getRootFallbackLink().getTargetFileSystem() + .getChildFileSystems())); + } + } catch (IOException ex) { + LOG.error("Could not add child filesystems for source path " + + fsState.getRootFallbackLink().fullPath + " with exception " + ex); } return children.toArray(new FileSystem[]{}); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystemOverloadScheme.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystemOverloadScheme.java index 773793ba3b..7dfd1eb948 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystemOverloadScheme.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystemOverloadScheme.java @@ -348,7 +348,7 @@ public MountPathInfo getMountPathInfo(Path path, FileSystem fs = res.isInternalDir() ? (fsState.getRootFallbackLink() != null ? ((ChRootedFileSystem) fsState - .getRootFallbackLink().targetFileSystem).getMyFs() : + .getRootFallbackLink().getTargetFileSystem()).getMyFs() : fsGetter().get(path.toUri(), conf)) : ((ChRootedFileSystem) res.targetFileSystem).getMyFs(); return new MountPathInfo(res.remainingPath, res.resolvedPath, @@ -390,8 +390,13 @@ public FileSystem getFallbackFileSystem() { if (fsState.getRootFallbackLink() == null) { return null; } - return ((ChRootedFileSystem) fsState.getRootFallbackLink().targetFileSystem) - .getMyFs(); + try { + return ((ChRootedFileSystem) fsState.getRootFallbackLink() + .getTargetFileSystem()).getMyFs(); + } catch (IOException ex) { + LOG.error("Could not get fallback filesystem "); + } + return null; } @Override diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java index a7d56fa56f..2aaba7e867 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java @@ -21,10 +21,12 @@ import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; +import java.util.function.Function; import java.io.FileNotFoundException; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.EnumSet; import java.util.HashSet; @@ -237,15 +239,32 @@ public ViewFs(final Configuration conf) throws IOException, initingUriAsFallbackOnNoMounts) { @Override - protected AbstractFileSystem getTargetFileSystem(final URI uri) - throws URISyntaxException, UnsupportedFileSystemException { - String pathString = uri.getPath(); - if (pathString.isEmpty()) { - pathString = "/"; + protected Function initAndGetTargetFs() { + return new Function() { + @Override + public AbstractFileSystem apply(final URI uri) { + AbstractFileSystem fs; + try { + fs = ugi.doAs( + new PrivilegedExceptionAction() { + @Override + public AbstractFileSystem run() throws IOException { + return AbstractFileSystem.createFileSystem(uri, config); + } + }); + String pathString = uri.getPath(); + if (pathString.isEmpty()) { + pathString = "/"; + } + return new ChRootedFs(fs, new Path(pathString)); + } catch (IOException | URISyntaxException | + InterruptedException ex) { + LOG.error("Could not initialize underlying FileSystem object" + +" for uri " + uri + "with exception: " + ex.toString()); + } + return null; } - return new ChRootedFs( - AbstractFileSystem.createFileSystem(uri, config), - new Path(pathString)); + }; } @Override @@ -719,7 +738,8 @@ public List> getDelegationTokens(String renewer) throws IOException { List> result = new ArrayList>(initialListSize); for ( int i = 0; i < mountPoints.size(); ++i ) { List> tokens = - mountPoints.get(i).target.targetFileSystem.getDelegationTokens(renewer); + mountPoints.get(i).target.getTargetFileSystem() + .getDelegationTokens(renewer); if (tokens != null) { result.addAll(tokens); } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestRegexMountPoint.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestRegexMountPoint.java index 5513b6005b..a5df2bab41 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestRegexMountPoint.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestRegexMountPoint.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.fs.viewfs; +import java.util.function.Function; import java.io.IOException; import java.net.URI; import java.util.Map; @@ -63,9 +64,14 @@ public void setUp() throws Exception { inodeTree = new InodeTree(conf, TestRegexMountPoint.class.getName(), null, false) { @Override - protected TestRegexMountPointFileSystem getTargetFileSystem( - final URI uri) { - return new TestRegexMountPointFileSystem(uri); + protected Function + initAndGetTargetFs() { + return new Function() { + @Override + public TestRegexMountPointFileSystem apply(URI uri) { + return new TestRegexMountPointFileSystem(uri); + } + }; } @Override diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsConfig.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsConfig.java index 56f5b2d997..7c318654ec 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsConfig.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsConfig.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.fs.viewfs; +import java.util.function.Function; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -42,7 +43,7 @@ class Foo { new InodeTree(conf, null, null, false) { @Override - protected Foo getTargetFileSystem(final URI uri) { + protected Function initAndGetTargetFs() { return null; } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index 037ea798c9..be50f457be 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -67,6 +67,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import static org.apache.hadoop.fs.FileSystemTestHelper.*; +import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_ENABLE_INNER_CACHE; import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555; import org.junit.After; @@ -1352,6 +1353,8 @@ public void testChildrenFileSystemLeak() throws Exception { final int cacheSize = TestFileUtil.getCacheSize(); ViewFileSystem viewFs = (ViewFileSystem) FileSystem .get(new URI("viewfs://" + clusterName + "/"), config); + viewFs.resolvePath( + new Path(String.format("viewfs://%s/%s", clusterName, "/user"))); assertEquals(cacheSize + 1, TestFileUtil.getCacheSize()); viewFs.close(); assertEquals(cacheSize, TestFileUtil.getCacheSize()); @@ -1428,4 +1431,45 @@ public void testGetContentSummaryWithFileInLocalFS() throws Exception { summaryAfter.getLength()); } } + + @Test + public void testTargetFileSystemLazyInitialization() throws Exception { + final String clusterName = "cluster" + new Random().nextInt(); + Configuration config = new Configuration(conf); + config.setBoolean(CONFIG_VIEWFS_ENABLE_INNER_CACHE, false); + config.setClass("fs.mockfs.impl", + TestChRootedFileSystem.MockFileSystem.class, FileSystem.class); + ConfigUtil.addLink(config, clusterName, "/user", + URI.create("mockfs://mockauth1/mockpath")); + ConfigUtil.addLink(config, clusterName, + "/mock", URI.create("mockfs://mockauth/mockpath")); + + final int cacheSize = TestFileUtil.getCacheSize(); + ViewFileSystem viewFs = (ViewFileSystem) FileSystem + .get(new URI("viewfs://" + clusterName + "/"), config); + + // As no inner file system instance has been initialized, + // cache size will remain the same + // cache is disabled for viewfs scheme, so the viewfs:// instance won't + // go in the cache even after the initialization + assertEquals(cacheSize, TestFileUtil.getCacheSize()); + + // This resolve path will initialize the file system corresponding + // to the mount table entry of the path "/user" + viewFs.resolvePath( + new Path(String.format("viewfs://%s/%s", clusterName, "/user"))); + + // Cache size will increase by 1. + assertEquals(cacheSize + 1, TestFileUtil.getCacheSize()); + // This resolve path will initialize the file system corresponding + // to the mount table entry of the path "/mock" + viewFs.resolvePath(new Path(String.format("viewfs://%s/%s", clusterName, + "/mock"))); + // One more file system instance will get initialized. + assertEquals(cacheSize + 2, TestFileUtil.getCacheSize()); + viewFs.close(); + // Initialized FileSystem instances will not be removed from cache as + // viewfs inner cache is disabled + assertEquals(cacheSize + 2, TestFileUtil.getCacheSize()); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java index b3836956c7..fcb52577d9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java @@ -21,8 +21,11 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.security.PrivilegedExceptionAction; import java.util.EnumSet; +import java.util.HashMap; +import java.util.Map; import javax.security.auth.login.LoginException; import org.apache.hadoop.conf.Configuration; @@ -39,6 +42,7 @@ import org.apache.hadoop.fs.FsShell; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.contract.ContractTestUtils; +import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.DFSTestUtil; import org.apache.hadoop.hdfs.MiniDFSCluster; @@ -46,6 +50,7 @@ import org.apache.hadoop.hdfs.client.CreateEncryptionZoneFlag; import org.apache.hadoop.hdfs.client.HdfsAdmin; import org.apache.hadoop.io.IOUtils; +import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.test.GenericTestUtils; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY; @@ -406,4 +411,72 @@ private void testNflyRepair(NflyFSystem.NflyKey repairKey) } } } + + @Test + public void testTargetFileSystemLazyInitializationWithUgi() throws Exception { + final Map map = new HashMap<>(); + final Path user1Path = new Path("/data/user1"); + + // Scenario - 1: Create FileSystem with the current user context + // Both mkdir and delete should be successful + FileSystem fs = FileSystem.get(FsConstants.VIEWFS_URI, conf); + fs.mkdirs(user1Path); + fs.delete(user1Path, false); + + // Scenario - 2: Create FileSystem with the a different user context + final UserGroupInformation userUgi = UserGroupInformation + .createUserForTesting("user1@HADOOP.COM", new String[]{"hadoop"}); + userUgi.doAs(new PrivilegedExceptionAction() { + @Override + public Object run() throws IOException { + UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); + String doAsUserName = ugi.getUserName(); + assertEquals(doAsUserName, "user1@HADOOP.COM"); + + FileSystem viewFS = FileSystem.get(FsConstants.VIEWFS_URI, conf); + map.put("user1", viewFS); + return null; + } + }); + + // Try creating a directory with the file context created by a different ugi + // Though we are running the mkdir with the current user context, the + // target filesystem will be instantiated by the ugi with which the + // file context was created. + try { + FileSystem otherfs = map.get("user1"); + otherfs.mkdirs(user1Path); + fail("This mkdir should fail"); + } catch (AccessControlException ex) { + // Exception is expected as the FileSystem was created with ugi of user1 + // So when we are trying to access the /user/user1 path for the first + // time, the corresponding file system is initialized and it tries to + // execute the task with ugi with which the FileSystem was created. + } + + // Change the permission of /data path so that user1 can create a directory + fsTarget.setOwner(new Path(targetTestRoot, "data"), + "user1", "test2"); + // set permission of target to allow rename to target + fsTarget.setPermission(new Path(targetTestRoot, "data"), + new FsPermission("775")); + + userUgi.doAs(new PrivilegedExceptionAction() { + @Override + public Object run() throws IOException { + FileSystem viewFS = FileSystem.get(FsConstants.VIEWFS_URI, conf); + map.put("user1", viewFS); + return null; + } + }); + + // Although we are running with current user context, and current user + // context does not have write permission, we are able to create the + // directory as its using ugi of user1 which has write permission. + FileSystem otherfs = map.get("user1"); + otherfs.mkdirs(user1Path); + String owner = otherfs.getFileStatus(user1Path).getOwner(); + assertEquals("The owner did not match ", owner, userUgi.getShortUserName()); + otherfs.delete(user1Path, false); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemOverloadSchemeWithHdfsScheme.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemOverloadSchemeWithHdfsScheme.java index 9a858e17eb..650a472279 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemOverloadSchemeWithHdfsScheme.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemOverloadSchemeWithHdfsScheme.java @@ -35,7 +35,6 @@ import org.apache.hadoop.fs.FsConstants; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.RawLocalFileSystem; -import org.apache.hadoop.fs.UnsupportedFileSystemException; import org.apache.hadoop.hdfs.DistributedFileSystem; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.test.LambdaTestUtils; @@ -206,7 +205,8 @@ public void testMountLinkWithNonExistentLink(boolean expectFsInitFailure) new String[] {nonExistTargetPath.toUri().toString()}, conf); if (expectFsInitFailure) { LambdaTestUtils.intercept(IOException.class, () -> { - FileSystem.get(conf); + FileSystem fs = FileSystem.get(conf); + fs.resolvePath(new Path(userFolder)); }); } else { try (FileSystem fs = FileSystem.get(conf)) { @@ -397,7 +397,7 @@ public void testCreateOnRoot(boolean fallbackExist) throws Exception { * Unset fs.viewfs.overload.scheme.target.hdfs.impl property. * So, OverloadScheme target fs initialization will fail. */ - @Test(expected = UnsupportedFileSystemException.class, timeout = 30000) + @Test(expected = IOException.class, timeout = 30000) public void testInvalidOverloadSchemeTargetFS() throws Exception { final Path hdfsTargetPath = new Path(defaultFSURI + HDFS_USER_FOLDER); String mountTableIfSet = conf.get(Constants.CONFIG_VIEWFS_MOUNTTABLE_PATH); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsHdfs.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsHdfs.java index fda667251b..540883dd2e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsHdfs.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFsHdfs.java @@ -21,18 +21,28 @@ import java.io.IOException; import java.net.URISyntaxException; +import java.security.PrivilegedExceptionAction; +import java.util.HashMap; +import java.util.Map; import javax.security.auth.login.LoginException; import org.apache.hadoop.fs.FileContext; import org.apache.hadoop.fs.FileContextTestHelper; +import org.apache.hadoop.fs.FsConstants; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.HdfsConfiguration; import org.apache.hadoop.hdfs.MiniDFSCluster; +import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; public class TestViewFsHdfs extends ViewFsBaseTest { @@ -85,5 +95,73 @@ public void setUp() throws Exception { int getExpectedDelegationTokenCount() { return 8; } + + @Test + public void testTargetFileSystemLazyInitialization() throws Exception { + final Map map = new HashMap<>(); + final Path user1Path = new Path("/data/user1"); + + // Scenario - 1: Create FileContext with the current user context + // Both mkdir and delete should be successful + FileContext fs = FileContext.getFileContext(FsConstants.VIEWFS_URI, conf); + fs.mkdir(user1Path, FileContext.DEFAULT_PERM, false); + fs.delete(user1Path, false); + + // Scenario - 2: Create FileContext with the a different user context + final UserGroupInformation userUgi = UserGroupInformation + .createUserForTesting("user1@HADOOP.COM", new String[]{"hadoop"}); + userUgi.doAs(new PrivilegedExceptionAction() { + @Override + public Object run() throws IOException { + UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); + String doAsUserName = ugi.getUserName(); + assertEquals(doAsUserName, "user1@HADOOP.COM"); + + FileContext viewFS = FileContext.getFileContext( + FsConstants.VIEWFS_URI, conf); + map.put("user1", viewFS); + return null; + } + }); + + // Try creating a directory with the file context created by a different ugi + // Though we are running the mkdir with the current user context, the + // target filesystem will be instantiated by the ugi with which the + // file context was created. + try { + FileContext otherfs = map.get("user1"); + otherfs.mkdir(user1Path, FileContext.DEFAULT_PERM, false); + fail("This mkdir should fail"); + } catch (AccessControlException ex) { + // Exception is expected as the FileContext was created with ugi of user1 + // So when we are trying to access the /user/user1 path for the first + // time, the corresponding file system is initialized and it tries to + // execute the task with ugi with which the FileContext was created. + } + + // Change the permission of /data path so that user1 can create a directory + fcView.setOwner(new Path("/data"), "user1", "test2"); + // set permission of target to allow rename to target + fcView.setPermission(new Path("/data"), new FsPermission("775")); + + userUgi.doAs(new PrivilegedExceptionAction() { + @Override + public Object run() throws IOException { + FileContext viewFS = FileContext.getFileContext( + FsConstants.VIEWFS_URI, conf); + map.put("user1", viewFS); + return null; + } + }); + + // Although we are running with current user context, and current user + // context does not have write permission, we are able to create the + // directory as its using ugi of user1 which has write permission. + FileContext otherfs = map.get("user1"); + otherfs.mkdir(user1Path, FileContext.DEFAULT_PERM, false); + String owner = otherfs.getFileStatus(user1Path).getOwner(); + assertEquals("The owner did not match ", owner, userUgi.getShortUserName()); + otherfs.delete(user1Path, false); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestViewFileSystemOverloadSchemeWithDFSAdmin.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestViewFileSystemOverloadSchemeWithDFSAdmin.java index 6119348f30..2f821cf277 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestViewFileSystemOverloadSchemeWithDFSAdmin.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestViewFileSystemOverloadSchemeWithDFSAdmin.java @@ -198,13 +198,15 @@ public void testSaveNamespaceWithoutSpecifyingFS() throws Exception { */ @Test public void testSafeModeWithWrongFS() throws Exception { + String wrongFsUri = "hdfs://nonExistent"; final Path hdfsTargetPath = - new Path("hdfs://nonExistent" + HDFS_USER_FOLDER); + new Path(wrongFsUri + HDFS_USER_FOLDER); addMountLinks(defaultFSURI.getHost(), new String[] {HDFS_USER_FOLDER}, new String[] {hdfsTargetPath.toUri().toString()}, conf); final DFSAdmin dfsAdmin = new DFSAdmin(conf); redirectStream(); - int ret = ToolRunner.run(dfsAdmin, new String[] {"-safemode", "enter" }); + int ret = ToolRunner.run(dfsAdmin, + new String[] {"-fs", wrongFsUri, "-safemode", "enter" }); assertEquals(-1, ret); assertErrMsg("safemode: java.net.UnknownHostException: nonExistent", 0); }