Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.7.2
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: fs/s3
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      S3a users System.getProperty("user.name") to get the username for the homedir. This is wrong, as it doesn't work on a YARN app where the identity is set by HADOOP_USER_NAME, or in a doAs clause.

      Obviously, UGI.getCurrentUser.getShortname() provides that name, everywhere.

      This is a simple change in the source, though testing is harder ... probably best to try in a doAs

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user steveloughran closed the pull request at:

          https://github.com/apache/hadoop/pull/136

          Show
          githubbot ASF GitHub Bot added a comment - Github user steveloughran closed the pull request at: https://github.com/apache/hadoop/pull/136
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10671 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10671/)
          HADOOP-12774. s3a should use UGI.getCurrentUser.getShortname() for (cnauroth: rev 3372e940303149d6258e0b72c54d72f080f0daa2)

          • (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java
          • (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java
          • (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java
          • (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10671 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10671/ ) HADOOP-12774 . s3a should use UGI.getCurrentUser.getShortname() for (cnauroth: rev 3372e940303149d6258e0b72c54d72f080f0daa2) (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for the patch. I did another test pass against us-west-2 to verify it. I have committed this to trunk, branch-2 and branch-2.8. Steve, thank you for the patch.

          Show
          cnauroth Chris Nauroth added a comment - +1 for the patch. I did another test pass against us-west-2 to verify it. I have committed this to trunk, branch-2 and branch-2.8. Steve, thank you for the patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 32s branch-2 passed
          +1 compile 0m 16s branch-2 passed with JDK v1.8.0_101
          +1 compile 0m 19s branch-2 passed with JDK v1.7.0_111
          +1 checkstyle 0m 15s branch-2 passed
          +1 mvnsite 0m 24s branch-2 passed
          +1 mvneclipse 0m 14s branch-2 passed
          +1 findbugs 0m 33s branch-2 passed
          +1 javadoc 0m 12s branch-2 passed with JDK v1.8.0_101
          +1 javadoc 0m 15s branch-2 passed with JDK v1.7.0_111
          +1 mvninstall 0m 17s the patch passed
          +1 compile 0m 13s the patch passed with JDK v1.8.0_101
          +1 javac 0m 13s the patch passed
          +1 compile 0m 15s the patch passed with JDK v1.7.0_111
          +1 javac 0m 15s the patch passed
          +1 checkstyle 0m 12s the patch passed
          +1 mvnsite 0m 21s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 44s the patch passed
          +1 javadoc 0m 10s the patch passed with JDK v1.8.0_101
          +1 javadoc 0m 14s the patch passed with JDK v1.7.0_111
          +1 unit 0m 21s hadoop-aws in the patch passed with JDK v1.7.0_111.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          14m 48s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue HADOOP-12774
          GITHUB PR https://github.com/apache/hadoop/pull/136
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f69ce4a0ad55 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / 5b7cbb5
          Default Java 1.7.0_111
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111
          findbugs v3.0.0
          JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10887/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10887/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 32s branch-2 passed +1 compile 0m 16s branch-2 passed with JDK v1.8.0_101 +1 compile 0m 19s branch-2 passed with JDK v1.7.0_111 +1 checkstyle 0m 15s branch-2 passed +1 mvnsite 0m 24s branch-2 passed +1 mvneclipse 0m 14s branch-2 passed +1 findbugs 0m 33s branch-2 passed +1 javadoc 0m 12s branch-2 passed with JDK v1.8.0_101 +1 javadoc 0m 15s branch-2 passed with JDK v1.7.0_111 +1 mvninstall 0m 17s the patch passed +1 compile 0m 13s the patch passed with JDK v1.8.0_101 +1 javac 0m 13s the patch passed +1 compile 0m 15s the patch passed with JDK v1.7.0_111 +1 javac 0m 15s the patch passed +1 checkstyle 0m 12s the patch passed +1 mvnsite 0m 21s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 44s the patch passed +1 javadoc 0m 10s the patch passed with JDK v1.8.0_101 +1 javadoc 0m 14s the patch passed with JDK v1.7.0_111 +1 unit 0m 21s hadoop-aws in the patch passed with JDK v1.7.0_111. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 14m 48s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue HADOOP-12774 GITHUB PR https://github.com/apache/hadoop/pull/136 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f69ce4a0ad55 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / 5b7cbb5 Default Java 1.7.0_111 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111 findbugs v3.0.0 JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10887/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10887/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          I submitted a fresh pre-commit run here:

          https://builds.apache.org/job/PreCommit-HADOOP-Build/10887/

          Show
          cnauroth Chris Nauroth added a comment - I submitted a fresh pre-commit run here: https://builds.apache.org/job/PreCommit-HADOOP-Build/10887/
          Hide
          stevel@apache.org Steve Loughran added a comment -

          submitting rebased on branch-2

          Show
          stevel@apache.org Steve Loughran added a comment - submitting rebased on branch-2
          Hide
          cnauroth Chris Nauroth added a comment -

          This needs to be rebased after the commit of HADOOP-13560.

          Show
          cnauroth Chris Nauroth added a comment - This needs to be rebased after the commit of HADOOP-13560 .
          Hide
          stevel@apache.org Steve Loughran added a comment -

          last patch in PR addresses the issues; Yetus somehow missed it.

          I'd like to hold off for HADOOP1-13560; IMO that one is ready for commit, and we'll have to see what problems arise downstream. FWIW I have been testing spark with it, not seen problems

          Show
          stevel@apache.org Steve Loughran added a comment - last patch in PR addresses the issues; Yetus somehow missed it. I'd like to hold off for HADOOP1-13560; IMO that one is ready for commit, and we'll have to see what problems arise downstream. FWIW I have been testing spark with it, not seen problems
          Hide
          cnauroth Chris Nauroth added a comment -

          +1, with removal of the unused import on commit.

          This patch is ready, but if we commit it now, then it will cause a conflict for HADOOP-13560. That's a big patch that is very close to commit, so I'd rather not force it to rebase. Steve Loughran, if you decide you want to commit this and then rebase HADOOP-13560, please go ahead. Otherwise, I'll wait until after HADOOP-13560 gets committed before I commit this one.

          Show
          cnauroth Chris Nauroth added a comment - +1, with removal of the unused import on commit. This patch is ready, but if we commit it now, then it will cause a conflict for HADOOP-13560 . That's a big patch that is very close to commit, so I'd rather not force it to rebase. Steve Loughran , if you decide you want to commit this and then rebase HADOOP-13560 , please go ahead. Otherwise, I'll wait until after HADOOP-13560 gets committed before I commit this one.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 9s branch-2 passed
          +1 compile 0m 15s branch-2 passed with JDK v1.8.0_101
          +1 compile 0m 18s branch-2 passed with JDK v1.7.0_111
          +1 checkstyle 0m 14s branch-2 passed
          +1 mvnsite 0m 23s branch-2 passed
          +1 mvneclipse 0m 14s branch-2 passed
          +1 findbugs 0m 35s branch-2 passed
          +1 javadoc 0m 12s branch-2 passed with JDK v1.8.0_101
          +1 javadoc 0m 16s branch-2 passed with JDK v1.7.0_111
          +1 mvninstall 0m 17s the patch passed
          +1 compile 0m 13s the patch passed with JDK v1.8.0_101
          +1 javac 0m 13s the patch passed
          +1 compile 0m 16s the patch passed with JDK v1.7.0_111
          +1 javac 0m 16s the patch passed
          +1 checkstyle 0m 13s the patch passed
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 42s the patch passed
          +1 javadoc 0m 10s the patch passed with JDK v1.8.0_101
          +1 javadoc 0m 13s the patch passed with JDK v1.7.0_111
          +1 unit 0m 20s hadoop-aws in the patch passed with JDK v1.7.0_111.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          15m 16s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue HADOOP-12774
          GITHUB PR https://github.com/apache/hadoop/pull/136
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 6a0770b7b1bb 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / a6197c1
          Default Java 1.7.0_111
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111
          findbugs v3.0.0
          JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10764/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10764/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 9s branch-2 passed +1 compile 0m 15s branch-2 passed with JDK v1.8.0_101 +1 compile 0m 18s branch-2 passed with JDK v1.7.0_111 +1 checkstyle 0m 14s branch-2 passed +1 mvnsite 0m 23s branch-2 passed +1 mvneclipse 0m 14s branch-2 passed +1 findbugs 0m 35s branch-2 passed +1 javadoc 0m 12s branch-2 passed with JDK v1.8.0_101 +1 javadoc 0m 16s branch-2 passed with JDK v1.7.0_111 +1 mvninstall 0m 17s the patch passed +1 compile 0m 13s the patch passed with JDK v1.8.0_101 +1 javac 0m 13s the patch passed +1 compile 0m 16s the patch passed with JDK v1.7.0_111 +1 javac 0m 16s the patch passed +1 checkstyle 0m 13s the patch passed +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 42s the patch passed +1 javadoc 0m 10s the patch passed with JDK v1.8.0_101 +1 javadoc 0m 13s the patch passed with JDK v1.7.0_111 +1 unit 0m 20s hadoop-aws in the patch passed with JDK v1.7.0_111. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 15m 16s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue HADOOP-12774 GITHUB PR https://github.com/apache/hadoop/pull/136 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 6a0770b7b1bb 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / a6197c1 Default Java 1.7.0_111 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111 findbugs v3.0.0 JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10764/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10764/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 36s branch-2 passed
          +1 compile 0m 18s branch-2 passed with JDK v1.8.0_101
          +1 compile 0m 18s branch-2 passed with JDK v1.7.0_111
          +1 checkstyle 0m 15s branch-2 passed
          +1 mvnsite 0m 24s branch-2 passed
          +1 mvneclipse 0m 18s branch-2 passed
          +1 findbugs 0m 34s branch-2 passed
          +1 javadoc 0m 13s branch-2 passed with JDK v1.8.0_101
          +1 javadoc 0m 16s branch-2 passed with JDK v1.7.0_111
          +1 mvninstall 0m 18s the patch passed
          +1 compile 0m 14s the patch passed with JDK v1.8.0_101
          +1 javac 0m 14s the patch passed
          +1 compile 0m 17s the patch passed with JDK v1.7.0_111
          +1 javac 0m 17s the patch passed
          -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 2 new + 9 unchanged - 0 fixed = 11 total (was 9)
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 43s the patch passed
          +1 javadoc 0m 10s the patch passed with JDK v1.8.0_101
          +1 javadoc 0m 13s the patch passed with JDK v1.7.0_111
          +1 unit 0m 20s hadoop-aws in the patch passed with JDK v1.7.0_111.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          14m 57s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue HADOOP-12774
          GITHUB PR https://github.com/apache/hadoop/pull/136
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 19b394eb8f95 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / f131d61
          Default Java 1.7.0_111
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10749/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10749/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10749/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 36s branch-2 passed +1 compile 0m 18s branch-2 passed with JDK v1.8.0_101 +1 compile 0m 18s branch-2 passed with JDK v1.7.0_111 +1 checkstyle 0m 15s branch-2 passed +1 mvnsite 0m 24s branch-2 passed +1 mvneclipse 0m 18s branch-2 passed +1 findbugs 0m 34s branch-2 passed +1 javadoc 0m 13s branch-2 passed with JDK v1.8.0_101 +1 javadoc 0m 16s branch-2 passed with JDK v1.7.0_111 +1 mvninstall 0m 18s the patch passed +1 compile 0m 14s the patch passed with JDK v1.8.0_101 +1 javac 0m 14s the patch passed +1 compile 0m 17s the patch passed with JDK v1.7.0_111 +1 javac 0m 17s the patch passed -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 2 new + 9 unchanged - 0 fixed = 11 total (was 9) +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 43s the patch passed +1 javadoc 0m 10s the patch passed with JDK v1.8.0_101 +1 javadoc 0m 13s the patch passed with JDK v1.7.0_111 +1 unit 0m 20s hadoop-aws in the patch passed with JDK v1.7.0_111. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 14m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue HADOOP-12774 GITHUB PR https://github.com/apache/hadoop/pull/136 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 19b394eb8f95 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / f131d61 Default Java 1.7.0_111 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10749/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10749/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10749/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          The latest pull request looks good to me. I think we still have an unused import of PrivilegedAction in ITestS3AConfiguration. That's easy to clean up on commit. I submitted another Jenkins run here:

          https://builds.apache.org/view/PreCommit%20Builds/job/PreCommit-HADOOP-Build/10749/

          Show
          cnauroth Chris Nauroth added a comment - The latest pull request looks good to me. I think we still have an unused import of PrivilegedAction in ITestS3AConfiguration . That's easy to clean up on commit. I submitted another Jenkins run here: https://builds.apache.org/view/PreCommit%20Builds/job/PreCommit-HADOOP-Build/10749/
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Patch 003: Address Chris's comments about using parent class fields. Also, given that the first contructor for S3AFileStatus is only for directories, the isdir parameter is always true, hence superflous. Cut and add javadocs for both constructors.

          Tested: s3a ireland, all well except for some intermittent consistency failures in ITestS3AContractRootDir.testRmEmptyRootDirNonRecursive. need to file a JIRA there

          Show
          stevel@apache.org Steve Loughran added a comment - Patch 003: Address Chris's comments about using parent class fields. Also, given that the first contructor for S3AFileStatus is only for directories, the isdir parameter is always true, hence superflous. Cut and add javadocs for both constructors. Tested: s3a ireland, all well except for some intermittent consistency failures in ITestS3AContractRootDir.testRmEmptyRootDirNonRecursive . need to file a JIRA there
          Hide
          cnauroth Chris Nauroth added a comment -

          Steve, I entered some comments on the pull request. In addition, Checkstyle flagged a few things that can be cleaned up. I needed to submit a pre-commit run manually at builds.apache.org to get that last report.

          Show
          cnauroth Chris Nauroth added a comment - Steve, I entered some comments on the pull request. In addition, Checkstyle flagged a few things that can be cleaned up. I needed to submit a pre-commit run manually at builds.apache.org to get that last report.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnauroth commented on a diff in the pull request:

          https://github.com/apache/hadoop/pull/136#discussion_r82690355

          — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java —
          @@ -32,18 +32,24 @@
          @InterfaceStability.Evolving
          public class S3AFileStatus extends FileStatus {
          private boolean isEmptyDirectory;
          + private final String owner;

          // Directories

          • public S3AFileStatus(boolean isdir, boolean isemptydir, Path path) {
            + public S3AFileStatus(boolean isdir,
            + boolean isemptydir,
            + Path path,
            + String owner) { super(0, isdir, 1, 0, 0, path); isEmptyDirectory = isemptydir; + this.owner = owner; }

          // Files
          public S3AFileStatus(long length, long modification_time, Path path,

          • long blockSize) {
            + long blockSize, String owner) {
            super(length, false, 1, blockSize, modification_time, path);
            isEmptyDirectory = false;
            + this.owner = owner;
              • End diff –

          Similar to the earlier comment:

          this.setOwner(owner);
          this.setGroup(owner);

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnauroth commented on a diff in the pull request: https://github.com/apache/hadoop/pull/136#discussion_r82690355 — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java — @@ -32,18 +32,24 @@ @InterfaceStability.Evolving public class S3AFileStatus extends FileStatus { private boolean isEmptyDirectory; + private final String owner; // Directories public S3AFileStatus(boolean isdir, boolean isemptydir, Path path) { + public S3AFileStatus(boolean isdir, + boolean isemptydir, + Path path, + String owner) { super(0, isdir, 1, 0, 0, path); isEmptyDirectory = isemptydir; + this.owner = owner; } // Files public S3AFileStatus(long length, long modification_time, Path path, long blockSize) { + long blockSize, String owner) { super(length, false, 1, blockSize, modification_time, path); isEmptyDirectory = false; + this.owner = owner; End diff – Similar to the earlier comment: this.setOwner(owner); this.setGroup(owner);
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnauroth commented on a diff in the pull request:

          https://github.com/apache/hadoop/pull/136#discussion_r82688916

          — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java —
          @@ -52,7 +58,16 @@ public boolean isEmptyDirectory() {

          @Override
          public String getOwner()

          { - return System.getProperty("user.name"); + return owner; + }

          +
          + /**
          + * The group of an S3A entry is the same as the owner
          + * @return the owner.
          + */
          + @Override
          + public String getGroup() {
          — End diff –

          The override of `getGroup` could be removed too.

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnauroth commented on a diff in the pull request: https://github.com/apache/hadoop/pull/136#discussion_r82688916 — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java — @@ -52,7 +58,16 @@ public boolean isEmptyDirectory() { @Override public String getOwner() { - return System.getProperty("user.name"); + return owner; + } + + /** + * The group of an S3A entry is the same as the owner + * @return the owner. + */ + @Override + public String getGroup() { — End diff – The override of `getGroup` could be removed too.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnauroth commented on a diff in the pull request:

          https://github.com/apache/hadoop/pull/136#discussion_r82688403

          — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java —
          @@ -32,18 +32,24 @@
          @InterfaceStability.Evolving
          public class S3AFileStatus extends FileStatus {
          private boolean isEmptyDirectory;
          + private final String owner;
          — End diff –

          I think we can achieve this change without adding a member variable in the subclass. (See more specific notes to follow.) Removing this member variable would reduce memory footprint. Admittedly, the memory cost is probably not significant, but as we start thinking about the possibility of caching `FileStatus` instances client-side for things like S3Guard, then the per-instance memory cost of each `FileStatus` could become more significant.

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnauroth commented on a diff in the pull request: https://github.com/apache/hadoop/pull/136#discussion_r82688403 — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java — @@ -32,18 +32,24 @@ @InterfaceStability.Evolving public class S3AFileStatus extends FileStatus { private boolean isEmptyDirectory; + private final String owner; — End diff – I think we can achieve this change without adding a member variable in the subclass. (See more specific notes to follow.) Removing this member variable would reduce memory footprint. Admittedly, the memory cost is probably not significant, but as we start thinking about the possibility of caching `FileStatus` instances client-side for things like S3Guard, then the per-instance memory cost of each `FileStatus` could become more significant.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnauroth commented on a diff in the pull request:

          https://github.com/apache/hadoop/pull/136#discussion_r82688624

          — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java —
          @@ -32,18 +32,24 @@
          @InterfaceStability.Evolving
          public class S3AFileStatus extends FileStatus {
          private boolean isEmptyDirectory;
          + private final String owner;

          // Directories

          • public S3AFileStatus(boolean isdir, boolean isemptydir, Path path) {
            + public S3AFileStatus(boolean isdir,
            + boolean isemptydir,
            + Path path,
            + String owner) {
            super(0, isdir, 1, 0, 0, path);
            isEmptyDirectory = isemptydir;
            + this.owner = owner;
              • End diff –

          Consider removing the member variable and changing this line of code to:

          this.setOwner(owner);
          this.setGroup(owner);

          (These are `protected` methods in the base class.)

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnauroth commented on a diff in the pull request: https://github.com/apache/hadoop/pull/136#discussion_r82688624 — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java — @@ -32,18 +32,24 @@ @InterfaceStability.Evolving public class S3AFileStatus extends FileStatus { private boolean isEmptyDirectory; + private final String owner; // Directories public S3AFileStatus(boolean isdir, boolean isemptydir, Path path) { + public S3AFileStatus(boolean isdir, + boolean isemptydir, + Path path, + String owner) { super(0, isdir, 1, 0, 0, path); isEmptyDirectory = isemptydir; + this.owner = owner; End diff – Consider removing the member variable and changing this line of code to: this.setOwner(owner); this.setGroup(owner); (These are `protected` methods in the base class.)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnauroth commented on a diff in the pull request:

          https://github.com/apache/hadoop/pull/136#discussion_r82689328

          — Diff: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java —
          @@ -42,8 +42,11 @@

          import java.io.File;
          import java.net.URI;
          +import java.security.PrivilegedAction;
          — End diff –

          Unused import?

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnauroth commented on a diff in the pull request: https://github.com/apache/hadoop/pull/136#discussion_r82689328 — Diff: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java — @@ -42,8 +42,11 @@ import java.io.File; import java.net.URI; +import java.security.PrivilegedAction; — End diff – Unused import?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnauroth commented on a diff in the pull request:

          https://github.com/apache/hadoop/pull/136#discussion_r82688875

          — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java —
          @@ -52,7 +58,16 @@ public boolean isEmptyDirectory() {

          @Override
          public String getOwner() {
          — End diff –

          If we implement the above comments, then we can completely remove the override of `getOwner` here.

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnauroth commented on a diff in the pull request: https://github.com/apache/hadoop/pull/136#discussion_r82688875 — Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java — @@ -52,7 +58,16 @@ public boolean isEmptyDirectory() { @Override public String getOwner() { — End diff – If we implement the above comments, then we can completely remove the override of `getOwner` here.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 33s branch-2 passed
          +1 compile 0m 16s branch-2 passed with JDK v1.8.0_101
          +1 compile 0m 19s branch-2 passed with JDK v1.7.0_111
          +1 checkstyle 0m 14s branch-2 passed
          +1 mvnsite 0m 25s branch-2 passed
          +1 mvneclipse 0m 14s branch-2 passed
          +1 findbugs 0m 35s branch-2 passed
          +1 javadoc 0m 13s branch-2 passed with JDK v1.8.0_101
          +1 javadoc 0m 16s branch-2 passed with JDK v1.7.0_111
          +1 mvninstall 0m 18s the patch passed
          +1 compile 0m 14s the patch passed with JDK v1.8.0_101
          +1 javac 0m 14s the patch passed
          +1 compile 0m 17s the patch passed with JDK v1.7.0_111
          +1 javac 0m 17s the patch passed
          -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 3 new + 9 unchanged - 0 fixed = 12 total (was 9)
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 44s the patch passed
          +1 javadoc 0m 11s the patch passed with JDK v1.8.0_101
          +1 javadoc 0m 14s the patch passed with JDK v1.7.0_111
          +1 unit 0m 22s hadoop-aws in the patch passed with JDK v1.7.0_111.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          15m 4s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue HADOOP-12774
          GITHUB PR https://github.com/apache/hadoop/pull/136
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux dd5983efa5c3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / de30f13
          Default Java 1.7.0_111
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10723/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10723/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10723/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 33s branch-2 passed +1 compile 0m 16s branch-2 passed with JDK v1.8.0_101 +1 compile 0m 19s branch-2 passed with JDK v1.7.0_111 +1 checkstyle 0m 14s branch-2 passed +1 mvnsite 0m 25s branch-2 passed +1 mvneclipse 0m 14s branch-2 passed +1 findbugs 0m 35s branch-2 passed +1 javadoc 0m 13s branch-2 passed with JDK v1.8.0_101 +1 javadoc 0m 16s branch-2 passed with JDK v1.7.0_111 +1 mvninstall 0m 18s the patch passed +1 compile 0m 14s the patch passed with JDK v1.8.0_101 +1 javac 0m 14s the patch passed +1 compile 0m 17s the patch passed with JDK v1.7.0_111 +1 javac 0m 17s the patch passed -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 3 new + 9 unchanged - 0 fixed = 12 total (was 9) +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 44s the patch passed +1 javadoc 0m 11s the patch passed with JDK v1.8.0_101 +1 javadoc 0m 14s the patch passed with JDK v1.7.0_111 +1 unit 0m 22s hadoop-aws in the patch passed with JDK v1.7.0_111. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 15m 4s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue HADOOP-12774 GITHUB PR https://github.com/apache/hadoop/pull/136 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux dd5983efa5c3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / de30f13 Default Java 1.7.0_111 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10723/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10723/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10723/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user steveloughran commented on the issue:

          https://github.com/apache/hadoop/pull/136

          Patch 002: create a fake user, create an FS and verify that the user and owner on the root path is that username

          Show
          githubbot ASF GitHub Bot added a comment - Github user steveloughran commented on the issue: https://github.com/apache/hadoop/pull/136 Patch 002: create a fake user, create an FS and verify that the user and owner on the root path is that username
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Created a PR which uses the current user at time of FS initialization shortname for the user and group on all FileStatus instances created during the FS life.

          Testing: all of s3a tests against s3a ireland, and the command line:

          $ ./hadoop fs -ls s3a://hwdev-steve-ireland/
          Found 1 items
          drwxrwxrwx   - stevel stevel          0 2016-10-07 17:29 s3a://hwdev-steve-ireland/tests
          
          Show
          stevel@apache.org Steve Loughran added a comment - Created a PR which uses the current user at time of FS initialization shortname for the user and group on all FileStatus instances created during the FS life. Testing: all of s3a tests against s3a ireland, and the command line: $ ./hadoop fs -ls s3a: //hwdev-steve-ireland/ Found 1 items drwxrwxrwx - stevel stevel 0 2016-10-07 17:29 s3a: //hwdev-steve-ireland/tests
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user steveloughran opened a pull request:

          https://github.com/apache/hadoop/pull/136

          HADOOP-12774 use UGI.currentUser for user and group of s3a objects

          This patch grabs the UGI current user shortname in the FS initialize call, then uses that as the user and group for all filestatus instances generated.

          ```
          $ ./hadoop fs -ls s3a://hwdev-steve-ireland/
          Found 1 items
          drwxrwxrwx - stevel stevel 0 2016-10-07 17:29 s3a://hwdev-steve-ireland/tests
          ```

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/steveloughran/hadoop s3/HADOOP-12774-username

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/hadoop/pull/136.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #136


          commit a58ed24a4b618fd3349eac40ad5900cfbd83faa3
          Author: Steve Loughran <stevel@apache.org>
          Date: 2016-10-07T16:30:25Z

          HADOOP-12774 use UGI.currentUser for user and group of s3a objects


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user steveloughran opened a pull request: https://github.com/apache/hadoop/pull/136 HADOOP-12774 use UGI.currentUser for user and group of s3a objects This patch grabs the UGI current user shortname in the FS initialize call, then uses that as the user and group for all filestatus instances generated. ``` $ ./hadoop fs -ls s3a://hwdev-steve-ireland/ Found 1 items drwxrwxrwx - stevel stevel 0 2016-10-07 17:29 s3a://hwdev-steve-ireland/tests ``` You can merge this pull request into a Git repository by running: $ git pull https://github.com/steveloughran/hadoop s3/ HADOOP-12774 -username Alternatively you can review and apply these changes as the patch at: https://github.com/apache/hadoop/pull/136.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #136 commit a58ed24a4b618fd3349eac40ad5900cfbd83faa3 Author: Steve Loughran <stevel@apache.org> Date: 2016-10-07T16:30:25Z HADOOP-12774 use UGI.currentUser for user and group of s3a objects
          Hide
          jzhuge John Zhuge added a comment -

          Do not have any bandwidth now. Unassigned.

          Show
          jzhuge John Zhuge added a comment - Do not have any bandwidth now. Unassigned.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          John, do you have a patch here? if not, can you unassign it?

          Show
          stevel@apache.org Steve Loughran added a comment - John, do you have a patch here? if not, can you unassign it?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          probably

          Show
          stevel@apache.org Steve Loughran added a comment - probably
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for reporting this, Steve Loughran.

          I'm wondering whether S3AFileStatus#getOwner() should be updated here as well?

          Show
          liuml07 Mingliang Liu added a comment - Thanks for reporting this, Steve Loughran . I'm wondering whether S3AFileStatus#getOwner() should be updated here as well?

            People

            • Assignee:
              stevel@apache.org Steve Loughran
              Reporter:
              stevel@apache.org Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development