From 12c0ace90730bf6cb408e96310b263ed9927ded5 Mon Sep 17 00:00:00 2001 From: Abhishek Das Date: Thu, 17 Feb 2022 20:16:19 -0800 Subject: [PATCH 1/2] HADOOP-18129: Change URI to String in INodeLink to reduce memory footprint of ViewFileSystem --- .../org/apache/hadoop/fs/shell/FsUsage.java | 6 ++-- .../apache/hadoop/fs/viewfs/InodeTree.java | 28 +++++++++---------- .../hadoop/fs/viewfs/ViewFileSystem.java | 10 +++---- .../org/apache/hadoop/fs/viewfs/ViewFs.java | 10 ++++--- .../fs/viewfs/ViewFileSystemBaseTest.java | 20 ++++++++++++- .../hadoop/hdfs/nfs/nfs3/Nfs3Utils.java | 3 +- 6 files changed, 49 insertions(+), 28 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java index 64aade3df9539..e29e4ec3a4546 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java @@ -140,12 +140,12 @@ protected void processPath(PathData item) throws IOException { FsStatus fsStatus = entry.getValue(); // Add the viewfs mount point status to report - URI[] mountPointFileSystemURIs = - viewFsMountPoint.getTargetFileSystemURIs(); + String[] mountPointFileSystemURIs = + viewFsMountPoint.getTargetFileSystemPaths(); // Since LinkMerge is not supported yet, we // should ideally see mountPointFileSystemURIs // array with only one element. - addToUsagesTable(mountPointFileSystemURIs[0], + addToUsagesTable(URI.create(mountPointFileSystemURIs[0]), fsStatus, viewFsMountPoint.getMountedOnPath().toString()); } } else { 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 bdc2b4e9d74b7..ef40f98cc02e0 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); @@ -677,7 +677,7 @@ protected InodeTree(final Configuration config, final String viewName, .info("Empty mount table detected for {} and considering itself " + "as a linkFallback.", theUri); 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 8c3cdb82d7447..722d7dfab75bd 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 @@ -216,19 +216,19 @@ 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() { return mountedOnPath; } - public URI[] getTargetFileSystemURIs() { - return targetFileSystemURIs; + 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 deaf4f4108a3e..6d619b1d6779d 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 91a90751e873e..ecfeadea69b28 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 @@ -175,7 +175,7 @@ public void testGetMountPoints() { MountPoint[] mountPoints = viewfs.getMountPoints(); for (MountPoint mountPoint : mountPoints) { LOG.info("MountPoint: " + mountPoint.getMountedOnPath() + " => " - + mountPoint.getTargetFileSystemURIs()[0]); + + mountPoint.getTargetFileSystemPaths()[0]); } Assert.assertEquals(getExpectedMountPoints(), mountPoints.length); } @@ -1671,4 +1671,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")); + } + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java index c58dc5976b37d..8f726deaf7284 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java +++ b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java @@ -256,7 +256,8 @@ public static URI getResolvedURI(FileSystem fs, String exportPath) String mountedPath = mount.getMountedOnPath().toString(); if (exportPath.startsWith(mountedPath)) { String subpath = exportPath.substring(mountedPath.length()); - fsURI = mount.getTargetFileSystemURIs()[0].resolve(subpath); + fsURI = URI.create(mount.getTargetFileSystemPaths()[0]) + .resolve(subpath); break; } } From d69c720598a2e5d8eadb145bfc3b8b47b6d16bd9 Mon Sep 17 00:00:00 2001 From: Abhishek Das Date: Mon, 14 Mar 2022 02:23:52 -0700 Subject: [PATCH 2/2] HADOOP-18129: Addressed code review comments --- .../src/main/java/org/apache/hadoop/fs/shell/FsUsage.java | 6 +++--- .../java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 8 ++++++++ .../apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java | 2 +- .../java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java | 3 +-- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java index e29e4ec3a4546..64aade3df9539 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java @@ -140,12 +140,12 @@ protected void processPath(PathData item) throws IOException { FsStatus fsStatus = entry.getValue(); // Add the viewfs mount point status to report - String[] mountPointFileSystemURIs = - viewFsMountPoint.getTargetFileSystemPaths(); + URI[] mountPointFileSystemURIs = + viewFsMountPoint.getTargetFileSystemURIs(); // Since LinkMerge is not supported yet, we // should ideally see mountPointFileSystemURIs // array with only one element. - addToUsagesTable(URI.create(mountPointFileSystemURIs[0]), + addToUsagesTable(mountPointFileSystemURIs[0], fsStatus, viewFsMountPoint.getMountedOnPath().toString()); } } else { 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 722d7dfab75bd..8c7580b1a38f9 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 @@ -227,6 +227,14 @@ public Path getMountedOnPath() { return mountedOnPath; } + public URI[] getTargetFileSystemURIs() { + 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/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 ecfeadea69b28..58180a8892d91 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 @@ -175,7 +175,7 @@ public void testGetMountPoints() { MountPoint[] mountPoints = viewfs.getMountPoints(); for (MountPoint mountPoint : mountPoints) { LOG.info("MountPoint: " + mountPoint.getMountedOnPath() + " => " - + mountPoint.getTargetFileSystemPaths()[0]); + + mountPoint.getTargetFileSystemURIs()[0]); } Assert.assertEquals(getExpectedMountPoints(), mountPoints.length); } diff --git a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java index 8f726deaf7284..c58dc5976b37d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java +++ b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java @@ -256,8 +256,7 @@ public static URI getResolvedURI(FileSystem fs, String exportPath) String mountedPath = mount.getMountedOnPath().toString(); if (exportPath.startsWith(mountedPath)) { String subpath = exportPath.substring(mountedPath.length()); - fsURI = URI.create(mount.getTargetFileSystemPaths()[0]) - .resolve(subpath); + fsURI = mount.getTargetFileSystemURIs()[0].resolve(subpath); break; } }