umamaheswararao commented on a change in pull request #2019:
URL: https://github.com/apache/hadoop/pull/2019#discussion_r430611396
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java
##########
@@ -915,11 +915,24 @@ public FileStatus getFileLinkStatus(final Path f)
if (inode.isLink()) {
INodeLink<AbstractFileSystem> inodelink =
(INodeLink<AbstractFileSystem>) inode;
- result = new FileStatus(0, false, 0, 0, creationTime, creationTime,
+ Path linkedPath = new Path(inodelink.targetDirLinkList[0].toString());
+ ChRootedFs linkedFs = (ChRootedFs) inodelink.getTargetFileSystem();
+ try {
+ FileStatus status = linkedFs.getMyFs().getFileStatus(linkedPath);
+ result = new FileStatus(status.getLen(), false,
+ status.getReplication(), status.getBlockSize(),
+ status.getModificationTime(), status.getAccessTime(),
+ status.getPermission(), status.getOwner(), status.getGroup(),
+ inodelink.getTargetLink(),
+ new Path(inode.fullPath).makeQualified(
+ myUri, null));
+ } catch (FileNotFoundException ex) {
Review comment:
> similar comment regarding FileNotFoundException. I think in general,
it's better to match behavior of non-federated client. If a path does not
exist, just throw back FileNotFoundException.
I just verified symlinks. When target deleted, ls on symlink does not throw
FNFE. Instead it is converted to file link. I tested dir->dir link.
It seems this behavior is correct when compared with other fs. I tested on
my MAC.
Should this be fixed in federated clusters is necessary ? Could you please
validate this?
If we attempt to open that non existent link file, then we can throw out
exception. But ls seems to simply pass.
```
Work % mkdir linkTarget
Work % ln -s linkTarget linkSrc
Work % ls -l
lrwxr-xr-x 1 umagangumalla xxxx 10 May 26 11:08 linkSrc -> linkTarget
Work % rm -rf linkTarget
Work % ls -l
lrwxr-xr-x 1 umagangumalla xxxx 10 May 26 11:08 linkSrc -> linkTarget
Work % cd linkSrc
cd: no such file or directory: linkSrc
```
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1211,13 +1211,29 @@ public FileStatus getFileStatus(Path f) throws
IOException {
INode<FileSystem> inode = iEntry.getValue();
if (inode.isLink()) {
INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
-
- result[i++] = new FileStatus(0, false, 0, 0,
- creationTime, creationTime, PERMISSION_555,
- ugi.getShortUserName(), ugi.getPrimaryGroupName(),
- link.getTargetLink(),
- new Path(inode.fullPath).makeQualified(
- myUri, null));
+ // For MERGE or NFLY links, the first target link is considered
+ // for fetching the FileStatus with an assumption that the permission
+ // and the owner will be the same for all the target directories.
+ Path linkedPath = new Path(link.targetDirLinkList[0].toString());
Review comment:
instead of link.targetDirLinkList[0],
link.getTargetFileSystem().getUri() should work?
This might work for nfly also I think. Because nfly has its own
GetFileStatus impl, probably we should use that impl only instead of getting
one targetDirLinkList[0]
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewfsFileStatus.java
##########
@@ -56,38 +69,71 @@ public void testFileStatusSerialziation()
File infile = new File(TEST_DIR, testfilename);
final byte[] content = "dingos".getBytes();
- FileOutputStream fos = null;
- try {
- fos = new FileOutputStream(infile);
+ try (FileOutputStream fos = new FileOutputStream(infile)) {
fos.write(content);
- } finally {
- if (fos != null) {
- fos.close();
- }
}
assertEquals((long)content.length, infile.length());
Configuration conf = new Configuration();
ConfigUtil.addLink(conf, "/foo/bar/baz", TEST_DIR.toURI());
- FileSystem vfs = FileSystem.get(FsConstants.VIEWFS_URI, conf);
- assertEquals(ViewFileSystem.class, vfs.getClass());
- Path path = new Path("/foo/bar/baz", testfilename);
- FileStatus stat = vfs.getFileStatus(path);
- assertEquals(content.length, stat.getLen());
- ContractTestUtils.assertNotErasureCoded(vfs, path);
- assertTrue(path + " should have erasure coding unset in " +
- "FileStatus#toString(): " + stat,
- stat.toString().contains("isErasureCoded=false"));
-
- // check serialization/deserialization
- DataOutputBuffer dob = new DataOutputBuffer();
- stat.write(dob);
- DataInputBuffer dib = new DataInputBuffer();
- dib.reset(dob.getData(), 0, dob.getLength());
- FileStatus deSer = new FileStatus();
- deSer.readFields(dib);
- assertEquals(content.length, deSer.getLen());
- assertFalse(deSer.isErasureCoded());
+ try (FileSystem vfs = FileSystem.get(FsConstants.VIEWFS_URI, conf)) {
+ assertEquals(ViewFileSystem.class, vfs.getClass());
+ Path path = new Path("/foo/bar/baz", testfilename);
+ FileStatus stat = vfs.getFileStatus(path);
+ assertEquals(content.length, stat.getLen());
+ ContractTestUtils.assertNotErasureCoded(vfs, path);
+ assertTrue(path + " should have erasure coding unset in " +
+ "FileStatus#toString(): " + stat,
+ stat.toString().contains("isErasureCoded=false"));
+
+ // check serialization/deserialization
+ DataOutputBuffer dob = new DataOutputBuffer();
+ stat.write(dob);
+ DataInputBuffer dib = new DataInputBuffer();
+ dib.reset(dob.getData(), 0, dob.getLength());
+ FileStatus deSer = new FileStatus();
+ deSer.readFields(dib);
+ assertEquals(content.length, deSer.getLen());
+ assertFalse(deSer.isErasureCoded());
+ }
+ }
+
+ @Test
+ public void testListStatusACL()
+ throws IOException, URISyntaxException {
+ String testfilename = "testFileACL";
+ String childDirectoryName = "testDirectoryACL";
+ TEST_DIR.mkdirs();
+ File infile = new File(TEST_DIR, testfilename);
+ final byte[] content = "dingos".getBytes();
+
+ try (FileOutputStream fos = new FileOutputStream(infile)) {
+ fos.write(content);
+ }
+ assertEquals((long)content.length, infile.length());
+ File childDir = new File(TEST_DIR, childDirectoryName);
+ childDir.mkdirs();
+
+ Configuration conf = new Configuration();
+ ConfigUtil.addLink(conf, "/file", infile.toURI());
+ ConfigUtil.addLink(conf, "/dir", childDir.toURI());
+
+ try (FileSystem vfs = FileSystem.get(FsConstants.VIEWFS_URI, conf)) {
+ assertEquals(ViewFileSystem.class, vfs.getClass());
+ FileStatus[] statuses = vfs.listStatus(new Path("/"));
+
+ FileSystem localFs = FileSystem.getLocal(conf);
+ FileStatus fileStat = localFs.getFileStatus(new Path(infile.getPath()));
+ FileStatus dirStat = localFs.getFileStatus(new Path(childDir.getPath()));
+
+ for (FileStatus status : statuses) {
+ if (status.getPath().getName().equals("file")) {
+ assertEquals(fileStat.getPermission(), status.getPermission());
+ } else {
Review comment:
Can we change permission on target and assert whether its getting
changed permissions or simply link permissions?
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1211,13 +1211,29 @@ public FileStatus getFileStatus(Path f) throws
IOException {
INode<FileSystem> inode = iEntry.getValue();
if (inode.isLink()) {
INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
-
- result[i++] = new FileStatus(0, false, 0, 0,
- creationTime, creationTime, PERMISSION_555,
- ugi.getShortUserName(), ugi.getPrimaryGroupName(),
- link.getTargetLink(),
- new Path(inode.fullPath).makeQualified(
- myUri, null));
+ // For MERGE or NFLY links, the first target link is considered
+ // for fetching the FileStatus with an assumption that the permission
+ // and the owner will be the same for all the target directories.
+ Path linkedPath = new Path(link.targetDirLinkList[0].toString());
+ ChRootedFileSystem linkedFs = (ChRootedFileSystem)
+ link.getTargetFileSystem();
+ try {
+ FileStatus status = linkedFs.getMyFs().getFileStatus(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));
+ } catch (FileNotFoundException ex) {
+ result[i++] = new FileStatus(0, false, 0, 0,
+ creationTime, creationTime, PERMISSION_555,
+ ugi.getShortUserName(), ugi.getPrimaryGroupName(),
+ link.getTargetLink(),
+ new Path(inode.fullPath).makeQualified(
+ myUri, null));
Review comment:
For #2: The problem here I think we may not be able reset mount
automatically, so until some one checks file exists or do op on target, we will
not know whether the file exists or not. This will continue until user updates
mount points accordingly.
This can be possible when some one deletes the target directory directly but
not updated the mount tables accordingly. Please check one of my comment on
behavior of ls in MAC. Also we have other issue: [ isDir is
inconsistent](https://issues.apache.org/jira/browse/HDFS-15370?focusedCommentId=17116927&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17116927).
----------------------------------------------------------------
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]
