Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1322

Document umask in DistributedFileSystem#mkdirs javadocs

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.3-alpha
    • Component/s: None
    • Labels:
    • Hadoop Flags:
      Reviewed

      Description

      The javadoc of DFSClient.mkdirs() says that the permissions of created dir will be set to dirPermission, which is not done currently. This should talk about umask.

      1. HDFS-1322.patch
        0.9 kB
        Plamen Jeliazkov
      2. HDFS-1322.002.patch
        3 kB
        Colin Patrick McCabe
      3. HDFS-1322.001.patch
        3 kB
        Colin Patrick McCabe

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1256 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1256/)
          HDFS-1322. Document umask in DistributedFileSystem#mkdirs javadocs. Contributed by Colin Patrick McCabe (Revision 1408532)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1408532
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1256 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1256/ ) HDFS-1322 . Document umask in DistributedFileSystem#mkdirs javadocs. Contributed by Colin Patrick McCabe (Revision 1408532) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1408532 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1225 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1225/)
          HDFS-1322. Document umask in DistributedFileSystem#mkdirs javadocs. Contributed by Colin Patrick McCabe (Revision 1408532)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1408532
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1225 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1225/ ) HDFS-1322 . Document umask in DistributedFileSystem#mkdirs javadocs. Contributed by Colin Patrick McCabe (Revision 1408532) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1408532 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #35 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/35/)
          HDFS-1322. Document umask in DistributedFileSystem#mkdirs javadocs. Contributed by Colin Patrick McCabe (Revision 1408532)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1408532
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #35 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/35/ ) HDFS-1322 . Document umask in DistributedFileSystem#mkdirs javadocs. Contributed by Colin Patrick McCabe (Revision 1408532) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1408532 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3006 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3006/)
          HDFS-1322. Document umask in DistributedFileSystem#mkdirs javadocs. Contributed by Colin Patrick McCabe (Revision 1408532)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1408532
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3006 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3006/ ) HDFS-1322 . Document umask in DistributedFileSystem#mkdirs javadocs. Contributed by Colin Patrick McCabe (Revision 1408532) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1408532 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          Hide
          Eli Collins added a comment -

          I've committed this. Thanks Colin.

          Show
          Eli Collins added a comment - I've committed this. Thanks Colin.
          Hide
          Eli Collins added a comment -

          Test failure is unrelated.

          Show
          Eli Collins added a comment - Test failure is unrelated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12553220/HDFS-1322.002.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.balancer.TestBalancerWithEncryptedTransfer

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3491//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3491//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12553220/HDFS-1322.002.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.balancer.TestBalancerWithEncryptedTransfer +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3491//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3491//console This message is automatically generated.
          Hide
          Eli Collins added a comment -

          +1

          Show
          Eli Collins added a comment - +1
          Hide
          Colin Patrick McCabe added a comment -

          address nits

          Show
          Colin Patrick McCabe added a comment - address nits
          Hide
          Eli Collins added a comment -

          Nits:

          • FsPermission line 189 needs a "*"
          • Use @see FSPermission#applyUMask(FsPermission)

          otherwise +1

          Show
          Eli Collins added a comment - Nits: FsPermission line 189 needs a "*" Use @see FSPermission#applyUMask(FsPermission) otherwise +1
          Hide
          Colin Patrick McCabe added a comment -

          note: No tests are needed for this patch because it only changes comments.

          Show
          Colin Patrick McCabe added a comment - note: No tests are needed for this patch because it only changes comments.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12546625/HDFS-1322.001.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3235//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3235//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12546625/HDFS-1322.001.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3235//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3235//console This message is automatically generated.
          Hide
          Colin Patrick McCabe added a comment -
          • update documentation for DistributedFileSystem#mkdirs to reflect umask.
          Show
          Colin Patrick McCabe added a comment - update documentation for DistributedFileSystem#mkdirs to reflect umask.
          Hide
          Colin Patrick McCabe added a comment -

          reopening as a doc change

          Show
          Colin Patrick McCabe added a comment - reopening as a doc change
          Hide
          Todd Lipcon added a comment -

          Shouldn't we update the javadoc to be clearer, as Ravi mentioned?

          Show
          Todd Lipcon added a comment - Shouldn't we update the javadoc to be clearer, as Ravi mentioned?
          Hide
          Colin Patrick McCabe added a comment -

          As Suresh commented, this is working as designed.

          If you don't want your permissions to be altered by the umask, you can set your client's umask to 0 using this code:

          conf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "0");
          [... create FS using conf ...]
          

          Try man umask for more information about umask.

          Show
          Colin Patrick McCabe added a comment - As Suresh commented, this is working as designed. If you don't want your permissions to be altered by the umask, you can set your client's umask to 0 using this code: conf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "0" ); [... create FS using conf ...] Try man umask for more information about umask.
          Hide
          Suresh Srinivas added a comment -

          > DistributedFileSystem.mkdirs(dir, dirPermission) calls DFSClient.mkdirs(dir, dirPermission) to create directory and then set permissions to (dirPermission & ~umask)
          This is the intended behavior right!

          Read http://linux.die.net/man/2/umask

          The umask for HDFS is set in the configuration file. It is being applied as permission & ~umask correctly both by DFSClient#mkdirs() and DFSClient#create(...). Perhaps the documentation of mkdirs() and create() should be changed to mention how umask is obtained from configuration and applied to the permission that is passed by the user.

          Show
          Suresh Srinivas added a comment - > DistributedFileSystem.mkdirs(dir, dirPermission) calls DFSClient.mkdirs(dir, dirPermission) to create directory and then set permissions to (dirPermission & ~umask) This is the intended behavior right! Read http://linux.die.net/man/2/umask The umask for HDFS is set in the configuration file. It is being applied as permission & ~umask correctly both by DFSClient#mkdirs() and DFSClient#create(...). Perhaps the documentation of mkdirs() and create() should be changed to mention how umask is obtained from configuration and applied to the permission that is passed by the user.
          Hide
          Plamen Jeliazkov added a comment -

          This patch should fix that. But this is not clear whether this is a bug or not. It could be that the JavaDocs are out of date and that umask'ing the permissions was a valid operation.

          Anyway, here is the patch to set DFSClient's .mkdirs() method use the parameter permissions and NOT umasking them.

          Show
          Plamen Jeliazkov added a comment - This patch should fix that. But this is not clear whether this is a bug or not. It could be that the JavaDocs are out of date and that umask'ing the permissions was a valid operation. Anyway, here is the patch to set DFSClient's .mkdirs() method use the parameter permissions and NOT umasking them.

            People

            • Assignee:
              Colin Patrick McCabe
              Reporter:
              Ravi Gummadi
            • Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development