[jira] [Commented] (HADOOP-9438) LocalFileContext does not throw an exception on mkdir for already existing directory

["Steve Loughran (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-9438?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13704380#comment-13704380 ] 

Steve Loughran commented on HADOOP-9438:
----------------------------------------

looking at where the code uses this, its a few places in YARN that tend to go

{code}
if (!fc.exists(path)) {
 try {
 fc.mkdirs(path)
 //some stuff to set permissions up
 ...
 } catch (FileAlreadyExistsException fae) {
  //noop
}

{code}

This catch actually ignores a potentially serious problem -parent path elements not being directories

# all FileSystem implementations return true if, at the end of the operation, the directory exists. This isn't in the javadocs of {{FileSystem.mkdirs()}} yet, but I plan to add it. In fact, if you look in {{FSNameSystem.mkdirsInternal()}} the fact that mkdirs() is always expected to succeed is called out

{code}
      // all the users of mkdirs() are used to expect 'true' even if
      // a new directory is not created.
{code}

If you look at the implementations of {{FileSystem.mkdirs()}}  they will throw either {{FileAlreadyExistsException}} or {{ParentNotDirectoryException}}. Swallowing a {{FileAlreadyExistsException}} can hide a serious problem -which is why MAPREDUCE-5264 is needed.

regarding this patch, the interface definition needs to retain the fact that an FAE can be thrown, but that the reason for doing so is the same as for {{ParentNotDirectoryException}}.


                
> LocalFileContext does not throw an exception on mkdir for already existing directory
> ------------------------------------------------------------------------------------
>
>                 Key: HADOOP-9438
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9438
>             Project: Hadoop Common
>          Issue Type: Bug
>    Affects Versions: 2.0.3-alpha
>            Reporter: Robert Joseph Evans
>            Priority: Critical
>         Attachments: HADOOP-9438.20130501.1.patch, HADOOP-9438.20130521.1.patch, HADOOP-9438.patch, HADOOP-9438.patch
>
>
> according to 
> http://hadoop.apache.org/docs/current/api/org/apache/hadoop/fs/FileContext.html#mkdir%28org.apache.hadoop.fs.Path,%20org.apache.hadoop.fs.permission.FsPermission,%20boolean%29
> should throw a FileAlreadyExistsException if the directory already exists.
> I tested this and 
> {code}
> FileContext lfc = FileContext.getLocalFSFileContext(new Configuration());
> Path p = new Path("/tmp/bobby.12345");
> FsPermission cachePerms = new FsPermission((short) 0755);
> lfc.mkdir(p, cachePerms, false);
> lfc.mkdir(p, cachePerms, false);
> {code}
> never throws an exception.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira