ayushtkn commented on a change in pull request #2088:
URL: https://github.com/apache/hadoop/pull/2088#discussion_r443743633



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1258,40 +1260,52 @@ public FileStatus getFileStatus(Path f) throws 
IOException {
             FileStatus status =
                 ((ChRootedFileSystem)link.getTargetFileSystem())
                 .getMyFs().getFileStatus(new Path(linkedPath));
-            result[i++] = new FileStatus(status.getLen(), status.isDirectory(),
-                status.getReplication(), status.getBlockSize(),
-                status.getModificationTime(), status.getAccessTime(),
-                status.getPermission(), status.getOwner(), status.getGroup(),
-                null, path);
+            linkStatuses.add(
+                new FileStatus(status.getLen(), status.isDirectory(),
+                    status.getReplication(), status.getBlockSize(),
+                    status.getModificationTime(), status.getAccessTime(),
+                    status.getPermission(), status.getOwner(),
+                    status.getGroup(), null, path));
           } catch (FileNotFoundException ex) {
             LOG.warn("Cannot get one of the children's(" + path
                 + ")  target path(" + link.getTargetFileSystem().getUri()
                 + ") file status.", ex);
             throw ex;
           }
         } else {
-          result[i++] =
+          internalDirStatuses.add(
               new FileStatus(0, true, 0, 0, creationTime, creationTime,
                   PERMISSION_555, ugi.getShortUserName(),
-                  ugi.getPrimaryGroupName(), path);
+                  ugi.getPrimaryGroupName(), path));
         }
       }
+      FileStatus[] internalDirStatusesMergedWithFallBack = internalDirStatuses
+          .toArray(new FileStatus[internalDirStatuses.size()]);
       if (fallbackStatuses.length > 0) {
-        return consolidateFileStatuses(fallbackStatuses, result);
+        internalDirStatusesMergedWithFallBack =
+            merge(fallbackStatuses, internalDirStatusesMergedWithFallBack);
+      }
+
+      // we don't use target file status as we show the mount link as symlink.
+      if (showMountLinksAsSymlinks) {
+        return merge(internalDirStatusesMergedWithFallBack,
+            linkStatuses.toArray(new FileStatus[linkStatuses.size()]));

Review comment:
       >Currently it will not work because we will say /user/hive is readOnly. 
But the plan is to support and create /user/hive/warehouse2 in fallback fs. So, 
it make sense to get the permission/others from fallback dir and represent. 
Does this make sense? 
   
   In most cases it makes sense, For one case I still have a doubt.
   But say taking the same tree,  /user1/hive/warehouse/partition-0 is mounted. 
and the same partion-0 is available through fallback as well.
   if I perform any operation say mkdir(/user1/hive/warehouse/partition-0/file) 
this will go to mount point resolved path not to partion-0 of the fallback 
path. The above line while listing will show permissions of partion-0 from fall 
back link, when listing /user1/hive/warehouse, Correct? But actually the mount 
entry will always be used. The question is which case partion-0 of the fallback 
will be used above the partion-0 from the mount entry. Even when partion-0 has 
target path available.
   
   if say in the fallback partion-0 isn't a directory, it is a file, we will 
show it as a file on listing /user1/hive/warehouse/, but mkdir on 
(/user1/hive/warehouse/partition-0/file) will be success, Wouldn't it be a 
inconsistent behavior ? 
   
   I modified your test for my case : 
   ```
    @Test
     public void testLSOnLinkParentWithFallbackLinkWithSameMountDirectoryTree()
         throws Exception {
       Configuration conf = new Configuration();
       conf.setBoolean(Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS, true);
       ConfigUtil.addLink(conf, "/user1/hive/warehouse/partition-0",
           new Path(targetTestRoot.toString()).toUri());
       // Creating multiple directories path under the fallback directory.
       // "/user1/hive/warehouse/partition-0" directory already exists as
       // configured mount point.
       Path file1 = new Path(targetTestRoot,
           "fallbackDir/user1/hive/warehouse/partition-0"); // partition-0 in
       // fallback is a file  ---> Changed this...
       Path dir2 = new Path(targetTestRoot, 
"fallbackDir/user1/hive/warehouse1");
       fsTarget.create(file1).close();
       fsTarget.mkdirs(dir2);
       URI viewFsUri = new URI(FsConstants.VIEWFS_SCHEME,
           Constants.CONFIG_VIEWFS_DEFAULT_MOUNT_TABLE, "/", null, null);
   
       ConfigUtil
           .addLinkFallback(conf, new Path(targetTestRoot, 
"fallbackDir").toUri());
   
       try (FileSystem vfs = FileSystem.get(viewFsUri, conf)) {
         for (FileStatus stat : vfs.listStatus(
             new Path(viewFsUri.toString(), "/user1/hive/warehouse/"))) {
           if (file1.getName().equals(stat.getPath().getName())) {
             assertTrue(stat.isFile()); // listing says
             // /user1/hive/warehouse/partition-0 is a file.
             Path fileUnderDir = new Path(stat.getPath(), "check");
             assertTrue(vfs.mkdirs(fileUnderDir)); // Created a file under file?
   
           }
         }
       }
     }
   ```
   Is this behavior fine?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to