[ https://issues.apache.org/jira/browse/HDFS-7087?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14953815#comment-14953815 ]
Xiao Chen commented on HDFS-7087: --------------------------------- Thanks a lot Andrew for the review and comments! I have attached patch 002 to address your comments. {quote} Would appreciate javadoc and comments explaining the new methods, and also rationale for the new fake FileStatuses. {quote} Added to {{FSDirectory#createReservedStatuses}}, please see if they make sense to you. {quote} Any reason you set the sticky bit? {quote} I initially thought we should set it to indicate the dir is not intended for unprivileged deletion. On a second thought, this is not needed and confusing, so I removed it. {quote} The isExactReservedName checks, noticed there's no check for rename. This seems brittle in general since we have to make sure it's present in everything besides list. Is the intent of these checks to provide consistent behavior? Do we think this is necessary, or can we simply rely on the existing behavior? {quote} I agree that it's fragile because we have to protect everything besides list. But we can’t rely on the existing behavior, because currently it’s protected by {{getFileInfo}} while parsing from the shell - before this patch, getFileInfo returns null for /.reserved so that the command isn't get processed and hence never reach to FSxxxxOp (except mkdirs). Now that we have to enable {{getFileInfo}} in order for ls to work, I think we have to protect other commands by ourselves. Below is a sample stack trace of the getFileInfo call. {noformat} at org.apache.hadoop.io.retry.RetryInvocationHandler.invoke(RetryInvocationHandler.java:80) at com.sun.proxy.$Proxy19.getFileInfo(Unknown Source:-1) at org.apache.hadoop.hdfs.DFSClient.getFileInfo(DFSClient.java:1667) at org.apache.hadoop.hdfs.DistributedFileSystem$23.doCall(DistributedFileSystem.java:1269) at org.apache.hadoop.hdfs.DistributedFileSystem$23.doCall(DistributedFileSystem.java:1265) at org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81) at org.apache.hadoop.hdfs.DistributedFileSystem.getFileStatus(DistributedFileSystem.java:1265) at org.apache.hadoop.fs.Globber.getFileStatus(Globber.java:62) at org.apache.hadoop.fs.Globber.doGlob(Globber.java:270) at org.apache.hadoop.fs.Globber.glob(Globber.java:149) at org.apache.hadoop.fs.FileSystem.globStatus(FileSystem.java:1670) at org.apache.hadoop.fs.shell.PathData.expandAsGlob(PathData.java:326) at org.apache.hadoop.fs.shell.Command.expandArgument(Command.java:239) at org.apache.hadoop.fs.shell.Command.expandArguments(Command.java:222) at org.apache.hadoop.fs.shell.FsCommand.processRawArguments(FsCommand.java:103) at org.apache.hadoop.fs.shell.Command.run(Command.java:166) at org.apache.hadoop.fs.FsShell.run(FsShell.java:310) at org.apache.hadoop.hdfs.TestDFSShell.runCmd(TestDFSShell.java:971) {noformat} {quote} This enables listing /.reserved/raw/.reserved/, which is a little weird. {quote} Agree and good catch! Fixed. {quote} There are some acrobatics to get the cTime. Is this something we could set during FSImage loading? Would let us avoid the null check. {quote} Good idea, updated. > Ability to list /.reserved > -------------------------- > > Key: HDFS-7087 > URL: https://issues.apache.org/jira/browse/HDFS-7087 > Project: Hadoop HDFS > Issue Type: Improvement > Affects Versions: 2.6.0 > Reporter: Andrew Wang > Assignee: Xiao Chen > Attachments: HDFS-7087.001.patch, HDFS-7087.002.patch, > HDFS-7087.draft.patch > > > We have two special paths within /.reserved now, /.reserved/.inodes and > /.reserved/raw. It seems like we should be able to list /.reserved to see > them. -- This message was sent by Atlassian JIRA (v6.3.4#6332)