From c3a4ce8ee8bb3f1fd873b02a3775ed6374745d45 Mon Sep 17 00:00:00 2001 From: Abhishek Das Date: Thu, 17 Feb 2022 20:16:19 -0800 Subject: [PATCH] HADOOP-18129: Change URI to String in INodeLink to reduce memory footprint of ViewFileSystem Fixes #3996 --- .../apache/hadoop/fs/viewfs/InodeTree.java | 28 +++++++++---------- .../hadoop/fs/viewfs/ViewFileSystem.java | 16 ++++++++--- .../org/apache/hadoop/fs/viewfs/ViewFs.java | 10 ++++--- .../fs/viewfs/ViewFileSystemBaseTest.java | 18 ++++++++++++ 4 files changed, 50 insertions(+), 22 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 05a1e93fc4..6753997127 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 @@ -273,7 +273,7 @@ enum LinkType { * is changed later it is then ignored (a dir with null entries) */ public static class INodeLink extends INode { - final URI[] targetDirLinkList; + final String[] targetDirLinkList; private T targetFileSystem; // file system object created from the link. // Function to initialize file system. Only applicable for simple links private Function fileSystemInitMethod; @@ -283,7 +283,7 @@ public static class INodeLink extends INode { * Construct a mergeLink or nfly. */ INodeLink(final String pathToNode, final UserGroupInformation aUgi, - final T targetMergeFs, final URI[] aTargetDirLinkList) { + final T targetMergeFs, final String[] aTargetDirLinkList) { super(pathToNode, aUgi); targetFileSystem = targetMergeFs; targetDirLinkList = aTargetDirLinkList; @@ -294,11 +294,11 @@ public static class INodeLink extends INode { */ INodeLink(final String pathToNode, final UserGroupInformation aUgi, Function createFileSystemMethod, - final URI aTargetDirLink) { + final String aTargetDirLink) throws URISyntaxException { super(pathToNode, aUgi); targetFileSystem = null; - targetDirLinkList = new URI[1]; - targetDirLinkList[0] = aTargetDirLink; + targetDirLinkList = new String[1]; + targetDirLinkList[0] = new URI(aTargetDirLink).toString(); this.fileSystemInitMethod = createFileSystemMethod; } @@ -336,7 +336,8 @@ public T getTargetFileSystem() throws IOException { if (targetFileSystem != null) { return targetFileSystem; } - targetFileSystem = fileSystemInitMethod.apply(targetDirLinkList[0]); + targetFileSystem = + fileSystemInitMethod.apply(URI.create(targetDirLinkList[0])); if (targetFileSystem == null) { throw new IOException( "Could not initialize target File System for URI : " + @@ -404,7 +405,7 @@ private void createLink(final String src, final String target, switch (linkType) { case SINGLE: newLink = new INodeLink(fullPath, aUgi, - initAndGetTargetFs(), new URI(target)); + initAndGetTargetFs(), target); break; case SINGLE_FALLBACK: case MERGE_SLASH: @@ -413,10 +414,10 @@ private void createLink(final String src, final String target, throw new IllegalArgumentException("Unexpected linkType: " + linkType); case MERGE: case NFLY: - final URI[] targetUris = StringUtils.stringToURI( - StringUtils.getStrings(target)); + final String[] targetUris = StringUtils.getStrings(target); newLink = new INodeLink(fullPath, aUgi, - getTargetFileSystem(settings, targetUris), targetUris); + getTargetFileSystem(settings, StringUtils.stringToURI(targetUris)), + targetUris); break; default: throw new IllegalArgumentException(linkType + ": Infeasible linkType"); @@ -633,8 +634,7 @@ protected InodeTree(final Configuration config, final String viewName, if (isMergeSlashConfigured) { Preconditions.checkNotNull(mergeSlashTarget); root = new INodeLink(mountTableName, ugi, - initAndGetTargetFs(), - new URI(mergeSlashTarget)); + initAndGetTargetFs(), mergeSlashTarget); mountPoints.add(new MountPoint("/", (INodeLink) root)); rootFallbackLink = null; } else { @@ -652,7 +652,7 @@ protected InodeTree(final Configuration config, final String viewName, + "not allowed."); } fallbackLink = new INodeLink(mountTableName, ugi, - initAndGetTargetFs(), new URI(le.getTarget())); + initAndGetTargetFs(), le.getTarget()); continue; case REGEX: addRegexMountEntry(le); @@ -678,7 +678,7 @@ protected InodeTree(final Configuration config, final String viewName, .append(" and considering itself as a linkFallback."); FileSystem.LOG.info(msg.toString()); rootFallbackLink = new INodeLink(mountTableName, ugi, - initAndGetTargetFs(), theUri); + initAndGetTargetFs(), theUri.toString()); getRootDir().addFallbackLink(rootFallbackLink); } } 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 8f333d1506..f93d361f1d 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 @@ -214,11 +214,11 @@ public static class MountPoint { /** * Array of target FileSystem URIs. */ - private final URI[] targetFileSystemURIs; + private final String[] targetFileSystemPaths; - MountPoint(Path srcPath, URI[] targetFs) { + MountPoint(Path srcPath, String[] targetFs) { mountedOnPath = srcPath; - targetFileSystemURIs = targetFs; + targetFileSystemPaths = targetFs; } public Path getMountedOnPath() { @@ -226,7 +226,15 @@ public Path getMountedOnPath() { } public URI[] getTargetFileSystemURIs() { - return targetFileSystemURIs; + URI[] targetUris = new URI[targetFileSystemPaths.length]; + for (int i = 0; i < targetFileSystemPaths.length; i++) { + targetUris[i] = URI.create(targetFileSystemPaths[i]); + } + return targetUris; + } + + public String[] getTargetFileSystemPaths() { + return targetFileSystemPaths; } } 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 2aaba7e867..7642fea163 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 @@ -185,16 +185,18 @@ static AccessControlException readOnlyMountTable(final String operation, static public class MountPoint { - private Path src; // the src of the mount - private URI[] targets; // target of the mount; Multiple targets imply mergeMount - MountPoint(Path srcPath, URI[] targetURIs) { + // the src of the mount + private Path src; + // Target of the mount; Multiple targets imply mergeMount + private String[] targets; + MountPoint(Path srcPath, String[] targetURIs) { src = srcPath; targets = targetURIs; } Path getSrc() { return src; } - URI[] getTargets() { + String[] getTargets() { return targets; } } 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 e4e7b0e245..7672d50bc7 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 @@ -1517,4 +1517,22 @@ public void testTargetFileSystemLazyInitializationForChecksumMethods() // viewfs inner cache is disabled assertEquals(cacheSize + 1, TestFileUtil.getCacheSize()); } + + @Test + public void testInvalidMountPoints() throws Exception { + final String clusterName = "cluster" + new Random().nextInt(); + Configuration config = new Configuration(conf); + config.set(ConfigUtil.getConfigViewFsPrefix(clusterName) + "." + + Constants.CONFIG_VIEWFS_LINK + "." + "/invalidPath", + "othermockfs:|mockauth/mockpath"); + + try { + FileSystem viewFs = FileSystem.get( + new URI("viewfs://" + clusterName + "/"), config); + fail("FileSystem should not initialize. Should fail with IOException"); + } catch (IOException ex) { + assertTrue("Should get URISyntax Exception", + ex.getMessage().startsWith("URISyntax exception")); + } + } }