umamaheswararao commented on a change in pull request #2061:
URL: https://github.com/apache/hadoop/pull/2061#discussion_r436770666
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1202,19 +1202,18 @@ public FileStatus getFileStatus(Path f) throws
IOException {
INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
try {
String linkedPath = link.getTargetFileSystem().getUri().getPath();
- if("".equals(linkedPath)) {
+ if ("".equals(linkedPath)) {
linkedPath = "/";
}
FileStatus status =
- ((ChRootedFileSystem)link.getTargetFileSystem())
- .getMyFs().getFileStatus(new Path(linkedPath));
- result[i++] = new FileStatus(status.getLen(), false,
- status.getReplication(), status.getBlockSize(),
- status.getModificationTime(), status.getAccessTime(),
- status.getPermission(), status.getOwner(), status.getGroup(),
- link.getTargetLink(),
- new Path(inode.fullPath).makeQualified(
- myUri, null));
+ ((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(),
+ link.getTargetLink(),
+ new Path(inode.fullPath).makeQualified(myUri, null));
Review comment:
```
isDir==true --> It is a Directory
isDir==false --> Can be a file or Symlink. So to conclude further whether a
file or link
isDir==false and link==null --> it is file
isDir==false and link!=null --> it is a symlink
```
Here nio Files APIs return true for isDirectory API. But here we cannot make
that judgement with this information. I saw that 'l' part along with directory.
However, native filesystems seems to be capturing the info about target
filesystem and return isDir true based on that. It denotes as symlink along
with permission bits.
My original thought was to change GetFileStatus see
[comment](https://issues.apache.org/jira/browse/HADOOP-17060?focusedCommentId=17113760&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17113760)
as you thought here:
But after verifying tests on local mac, I realized isDirectory is getting
returned tru in that cases, but here we cannot make that decision. in MAC it
was showing as folder icon if target is a directory and isDirectory as true.
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1202,19 +1202,18 @@ public FileStatus getFileStatus(Path f) throws
IOException {
INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
try {
String linkedPath = link.getTargetFileSystem().getUri().getPath();
- if("".equals(linkedPath)) {
+ if ("".equals(linkedPath)) {
linkedPath = "/";
}
FileStatus status =
- ((ChRootedFileSystem)link.getTargetFileSystem())
- .getMyFs().getFileStatus(new Path(linkedPath));
- result[i++] = new FileStatus(status.getLen(), false,
- status.getReplication(), status.getBlockSize(),
- status.getModificationTime(), status.getAccessTime(),
- status.getPermission(), status.getOwner(), status.getGroup(),
- link.getTargetLink(),
- new Path(inode.fullPath).makeQualified(
- myUri, null));
+ ((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(),
+ link.getTargetLink(),
+ new Path(inode.fullPath).makeQualified(myUri, null));
Review comment:
```
isDir==true --> It is a Directory
isDir==false --> Can be a file or Symlink. So to conclude further whether a
file or link
isDir==false and link==null --> it is file
isDir==false and link!=null --> it is a symlink
```
Here nio Files APIs return true for isDirectory API. But here we cannot make
that judgement with this information. I saw that 'l' part along with directory.
However, native filesystems seems to be capturing the info about target
filesystem and return isDir true based on that. It denotes as symlink along
with permission bits.
My original thought was to change GetFileStatus see
[comment](https://issues.apache.org/jira/browse/HADOOP-17060?focusedCommentId=17113760&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17113760)
as you thought here:
But after verifying tests on local mac, I realized isDirectory is getting
returned tru in that cases, but here we cannot make that decision. in MAC it
was showing as folder icon if target is a directory and isDirectory as true.
Yes, they are incompatible changes( either the change done in
listStatus/GetFileStatus)( we will mark them), and behavioral corrections in
other side.
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1202,19 +1202,18 @@ public FileStatus getFileStatus(Path f) throws
IOException {
INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
try {
String linkedPath = link.getTargetFileSystem().getUri().getPath();
- if("".equals(linkedPath)) {
+ if ("".equals(linkedPath)) {
linkedPath = "/";
}
FileStatus status =
- ((ChRootedFileSystem)link.getTargetFileSystem())
- .getMyFs().getFileStatus(new Path(linkedPath));
- result[i++] = new FileStatus(status.getLen(), false,
- status.getReplication(), status.getBlockSize(),
- status.getModificationTime(), status.getAccessTime(),
- status.getPermission(), status.getOwner(), status.getGroup(),
- link.getTargetLink(),
- new Path(inode.fullPath).makeQualified(
- myUri, null));
+ ((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(),
+ link.getTargetLink(),
+ new Path(inode.fullPath).makeQualified(myUri, null));
Review comment:
From ur previous comment:
```
drwxrwxr-x 2 ayush ayush 4096 Jun 8 04:25 dir/
lrwxrwxrwx 1 ayush ayush 4 Jun 8 04:25 sym_dir -> dir//
-rw-rw-r-- 1 ayush ayush 0 Jun 8 11:05 txt
```
This is from permission bits. If you look at our FSPermission class we have
a stickybit to denote its a file or not. Unfortunately it's just a boolean, it
does not have 3rd option to represent link.
```
public FsPermission(FsAction u, FsAction g, FsAction o, boolean sb) {
set(u, g, o, sb);
}
```
(nio) Files.isDirectory returns true for a symlink if target is dir. Where
as Hadoop FileStatus class does not allow if symlink represent as dir.
Behaviors are confusing :-(
That means we don't have enough info captured in Permissions or FileStatus
classes to represent symlink properly.
Coming to getFileStatus question:
In your above quoted code runs on internal directory, thats not a link.
Here internal directory means, if you have link src as /a/b/c, here a, b are
internal dirs (here it's always a dir, so we can safely hardcode to true) and
c is a link dir.
GetFileStatus run in input path, so, when input path is link, it would have
resolved and run getFileStatus on targetFileSystem#getFileStatus. Where
listStatus gets the FileStatus of immediate childrens. So, listStatus of /a/b ,
since be is an internal dir, it will go to InetrnalViewFsDir and run
listStatus. and one of its child c is a link. so previously treating that
children FileStatus object as symlink with isDir is false, irrespective whether
target is file/dir. One safe assumption we can keep is, in ViewFS target would
always be a directory only. I don't think anyone would configure a file as
target link. But we can not leave that case anyway for consistency. Hope this
helps. I also don't have very clear opinions here. Symlinks seems to mess
around here. :-( .
But I like the idea of representing as symlink as that one of fs features.
but consistent behavior is what we need to find here. If we wanted to fix
getFileStatus("/a/b/c"), we need to make isDir as false, but current it will
return true as it runs on targetFileSystem and also give symlink with tagetFs
path. Here no way user know whether the target is file or dir, unless he
directly access symlinked path and see.
Let me catchup Sanjay, if he has some thoughts around.
`But in any case, I don't have oppositions to any of the approach, all up to
you whichever way you want to go ahead. :-)`
Thank you. Let's figure out what would be the correct thing to do. I have no
plans to move ahead until we find some reasonable solution here.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
