You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by om...@apache.org on 2022/03/18 00:28:13 UTC

[hadoop] branch branch-3.3 updated: HADOOP-18129: Change URI to String in INodeLink to reduce memory footprint of ViewFileSystem

This is an automated email from the ASF dual-hosted git repository.

omalley pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/branch-3.3 by this push:
     new c3a4ce8  HADOOP-18129: Change URI to String in INodeLink to reduce memory footprint of ViewFileSystem
c3a4ce8 is described below

commit c3a4ce8ee8bb3f1fd873b02a3775ed6374745d45
Author: Abhishek Das <ab...@gmail.com>
AuthorDate: Thu Feb 17 20:16:19 2022 -0800

    HADOOP-18129: Change URI to String in INodeLink to reduce memory footprint of ViewFileSystem
    
    Fixes #3996
---
 .../org/apache/hadoop/fs/viewfs/InodeTree.java     | 28 +++++++++++-----------
 .../apache/hadoop/fs/viewfs/ViewFileSystem.java    | 16 +++++++++----
 .../java/org/apache/hadoop/fs/viewfs/ViewFs.java   | 10 ++++----
 .../hadoop/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 05a1e93..6753997 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 @@ abstract class InodeTree<T> {
    * is changed later it is then ignored (a dir with null entries)
    */
   public static class INodeLink<T> extends INode<T> {
-    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<URI, T> fileSystemInitMethod;
@@ -283,7 +283,7 @@ abstract class InodeTree<T> {
      * 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 @@ abstract class InodeTree<T> {
      */
     INodeLink(final String pathToNode, final UserGroupInformation aUgi,
         Function<URI, T> 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 @@ abstract class InodeTree<T> {
           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 @@ abstract class InodeTree<T> {
     switch (linkType) {
     case SINGLE:
       newLink = new INodeLink<T>(fullPath, aUgi,
-          initAndGetTargetFs(), new URI(target));
+          initAndGetTargetFs(), target);
       break;
     case SINGLE_FALLBACK:
     case MERGE_SLASH:
@@ -413,10 +414,10 @@ abstract class InodeTree<T> {
       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<T>(fullPath, aUgi,
-            getTargetFileSystem(settings, targetUris), targetUris);
+          getTargetFileSystem(settings, StringUtils.stringToURI(targetUris)),
+          targetUris);
       break;
     default:
       throw new IllegalArgumentException(linkType + ": Infeasible linkType");
@@ -633,8 +634,7 @@ abstract class InodeTree<T> {
     if (isMergeSlashConfigured) {
       Preconditions.checkNotNull(mergeSlashTarget);
       root = new INodeLink<T>(mountTableName, ugi,
-          initAndGetTargetFs(),
-          new URI(mergeSlashTarget));
+          initAndGetTargetFs(), mergeSlashTarget);
       mountPoints.add(new MountPoint<T>("/", (INodeLink<T>) root));
       rootFallbackLink = null;
     } else {
@@ -652,7 +652,7 @@ abstract class InodeTree<T> {
                 + "not allowed.");
           }
           fallbackLink = new INodeLink<T>(mountTableName, ugi,
-              initAndGetTargetFs(), new URI(le.getTarget()));
+              initAndGetTargetFs(), le.getTarget());
           continue;
         case REGEX:
           addRegexMountEntry(le);
@@ -678,7 +678,7 @@ abstract class InodeTree<T> {
               .append(" and considering itself as a linkFallback.");
       FileSystem.LOG.info(msg.toString());
       rootFallbackLink = new INodeLink<T>(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 8f333d1..f93d361 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 class ViewFileSystem extends FileSystem {
     /**
      * 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 class ViewFileSystem extends FileSystem {
     }
 
     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 2aaba7e..7642fea 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 @@ public class ViewFs extends AbstractFileSystem {
   
   
   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 e4e7b0e..7672d50 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 @@ abstract public class ViewFileSystemBaseTest {
     // 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"));
+    }
+  }
 }

---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org