Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-9631

ViewFs should use underlying FileSystem's server side defaults

    Details

      Description

      On a cluster with ViewFS as default FileSystem, creating files using FileContext will always result with replication factor of 1, instead of underlying filesystem default (like HDFS)

      1. HADOOP-9631.005.patch
        14 kB
        Erik Krogen
      2. HADOOP-9631.006.patch
        15 kB
        Erik Krogen
      3. HADOOP-9631.007.patch
        16 kB
        Erik Krogen
      4. HADOOP-9631.trunk.1.patch
        14 kB
        Lohit Vijayarenu
      5. HADOOP-9631.trunk.2.patch
        3 kB
        Lohit Vijayarenu
      6. HADOOP-9631.trunk.3.patch
        16 kB
        Lohit Vijayarenu
      7. HADOOP-9631.trunk.4.patch
        15 kB
        Lohit Vijayarenu
      8. TestFileContext.java
        0.7 kB
        Lohit Vijayarenu

        Issue Links

          Activity

          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          2.8.1 became a security release. Moving fix-version to 2.8.2 after the fact.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - 2.8.1 became a security release. Moving fix-version to 2.8.2 after the fact.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11443 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11443/)
          HADOOP-9631. ViewFs should use underlying FileSystem's server side (zhz: rev 59d69257a888347f0fb9c51bb000afc986b64f98)

          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ChRootedFs.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/fs/Hdfs.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/DelegateToFileSystem.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFsBaseTest.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FtpFs.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFs.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFs.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestAfsCheckPath.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11443 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11443/ ) HADOOP-9631 . ViewFs should use underlying FileSystem's server side (zhz: rev 59d69257a888347f0fb9c51bb000afc986b64f98) (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ChRootedFs.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/fs/Hdfs.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/DelegateToFileSystem.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFsBaseTest.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FtpFs.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFs.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFs.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestAfsCheckPath.java
          Hide
          zhz Zhe Zhang added a comment -

          Committed to above mentioned branches. Thanks Lohit and Erik for the contribution!

          Show
          zhz Zhe Zhang added a comment - Committed to above mentioned branches. Thanks Lohit and Erik for the contribution!
          Hide
          zhz Zhe Zhang added a comment -

          Thanks Erik Krogen for the update! +1 on v7 patch. I just committed to trunk, working on backports. I think this should go into branch-2, branch-2.8, and branch-2.7.

          Show
          zhz Zhe Zhang added a comment - Thanks Erik Krogen for the update! +1 on v7 patch. I just committed to trunk, working on backports. I think this should go into branch-2, branch-2.8, and branch-2.7.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 42s Maven dependency ordering for branch
          +1 mvninstall 13m 3s trunk passed
          +1 compile 21m 27s trunk passed
          +1 checkstyle 1m 58s trunk passed
          +1 mvnsite 1m 43s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 3m 30s trunk passed
          +1 javadoc 1m 20s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 25s the patch passed
          +1 compile 17m 46s the patch passed
          +1 javac 17m 46s root generated 0 new + 777 unchanged - 1 fixed = 777 total (was 778)
          +1 checkstyle 2m 12s the patch passed
          +1 mvnsite 2m 3s the patch passed
          +1 mvneclipse 0m 45s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 45s the patch passed
          +1 javadoc 1m 24s the patch passed
          -1 unit 8m 31s hadoop-common in the patch failed.
          +1 unit 1m 10s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 40s The patch does not generate ASF License warnings.
          109m 11s



          Reason Tests
          Failed junit tests hadoop.security.TestKDiag



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-9631
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12860051/HADOOP-9631.007.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f73fff65cb25 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f462e1f
          Default Java 1.8.0_121
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11887/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11887/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11887/console
          Powered by Apache Yetus 0.5.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 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 42s Maven dependency ordering for branch +1 mvninstall 13m 3s trunk passed +1 compile 21m 27s trunk passed +1 checkstyle 1m 58s trunk passed +1 mvnsite 1m 43s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 3m 30s trunk passed +1 javadoc 1m 20s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 25s the patch passed +1 compile 17m 46s the patch passed +1 javac 17m 46s root generated 0 new + 777 unchanged - 1 fixed = 777 total (was 778) +1 checkstyle 2m 12s the patch passed +1 mvnsite 2m 3s the patch passed +1 mvneclipse 0m 45s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 45s the patch passed +1 javadoc 1m 24s the patch passed -1 unit 8m 31s hadoop-common in the patch failed. +1 unit 1m 10s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 40s The patch does not generate ASF License warnings. 109m 11s Reason Tests Failed junit tests hadoop.security.TestKDiag Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-9631 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12860051/HADOOP-9631.007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f73fff65cb25 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f462e1f Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11887/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11887/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11887/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xkrogen Erik Krogen added a comment -

          Zhe's comments addressed in v007 patch

          Show
          xkrogen Erik Krogen added a comment - Zhe's comments addressed in v007 patch
          Hide
          zhz Zhe Zhang added a comment -

          Thanks Erik Krogen for the work! v6 patch LGTM, with a couple of minor comments:

          1. ViewFs#getServerDefaults has unnecessary exceptions in signature
          2. Can we enhance the test to cover the case where ViewFs#getServerDefaults(f) where f is an internal dir?
          Show
          zhz Zhe Zhang added a comment - Thanks Erik Krogen for the work! v6 patch LGTM, with a couple of minor comments: ViewFs#getServerDefaults has unnecessary exceptions in signature Can we enhance the test to cover the case where ViewFs#getServerDefaults(f) where f is an internal dir?
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 1m 57s Maven dependency ordering for branch
          +1 mvninstall 13m 0s trunk passed
          +1 compile 21m 17s trunk passed
          +1 checkstyle 1m 58s trunk passed
          +1 mvnsite 1m 45s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 3m 16s trunk passed
          +1 javadoc 1m 19s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 13s the patch passed
          +1 compile 16m 35s the patch passed
          +1 javac 16m 35s root generated 0 new + 777 unchanged - 1 fixed = 777 total (was 778)
          +1 checkstyle 2m 2s the patch passed
          +1 mvnsite 1m 47s the patch passed
          +1 mvneclipse 0m 46s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 43s the patch passed
          +1 javadoc 1m 29s the patch passed
          +1 unit 8m 25s hadoop-common in the patch passed.
          +1 unit 1m 14s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 38s The patch does not generate ASF License warnings.
          108m 10s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-9631
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12860026/HADOOP-9631.006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 641114bbca38 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f462e1f
          Default Java 1.8.0_121
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11884/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11884/console
          Powered by Apache Yetus 0.5.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 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 1m 57s Maven dependency ordering for branch +1 mvninstall 13m 0s trunk passed +1 compile 21m 17s trunk passed +1 checkstyle 1m 58s trunk passed +1 mvnsite 1m 45s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 3m 16s trunk passed +1 javadoc 1m 19s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 13s the patch passed +1 compile 16m 35s the patch passed +1 javac 16m 35s root generated 0 new + 777 unchanged - 1 fixed = 777 total (was 778) +1 checkstyle 2m 2s the patch passed +1 mvnsite 1m 47s the patch passed +1 mvneclipse 0m 46s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 43s the patch passed +1 javadoc 1m 29s the patch passed +1 unit 8m 25s hadoop-common in the patch passed. +1 unit 1m 14s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 38s The patch does not generate ASF License warnings. 108m 10s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-9631 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12860026/HADOOP-9631.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 641114bbca38 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f462e1f Default Java 1.8.0_121 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11884/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11884/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xkrogen Erik Krogen added a comment -

          Forgot a try/catch... My mistake.

          One note - to maintain backwards compatibility, if you call ViewFs#getServerDefaults() (with no argument) or call it on one of the internal directories, you just get the LocalConfigKeys.getServerDefaults() (maintaining existing behavior). This is in contrast to ViewFileSystem's behavior in which either of these calls results in a NotInMountpointException.

          Show
          xkrogen Erik Krogen added a comment - Forgot a try/catch... My mistake. One note - to maintain backwards compatibility, if you call ViewFs#getServerDefaults() (with no argument) or call it on one of the internal directories, you just get the LocalConfigKeys.getServerDefaults() (maintaining existing behavior). This is in contrast to ViewFileSystem 's behavior in which either of these calls results in a NotInMountpointException .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s 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.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 15m 7s trunk passed
          +1 compile 23m 26s trunk passed
          +1 checkstyle 2m 16s trunk passed
          +1 mvnsite 2m 0s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 3m 35s trunk passed
          +1 javadoc 1m 34s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 14s the patch passed
          +1 compile 16m 54s the patch passed
          -1 javac 16m 54s root generated 1 new + 777 unchanged - 1 fixed = 778 total (was 778)
          +1 checkstyle 2m 3s the patch passed
          +1 mvnsite 1m 46s the patch passed
          +1 mvneclipse 0m 44s the patch passed
          -1 whitespace 0m 0s The patch has 9 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 3m 28s the patch passed
          +1 javadoc 1m 27s the patch passed
          -1 unit 8m 5s hadoop-common in the patch failed.
          +1 unit 1m 8s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 40s The patch does not generate ASF License warnings.
          111m 27s



          Reason Tests
          Failed junit tests hadoop.security.TestKDiag
            hadoop.fs.viewfs.TestViewFsWithAuthorityLocalFs
            hadoop.fs.viewfs.TestViewFsLocalFs



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-9631
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12859959/HADOOP-9631.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 534a3124848e 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f462e1f
          Default Java 1.8.0_121
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/artifact/patchprocess/diff-compile-javac-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/console
          Powered by Apache Yetus 0.5.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 17s 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. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 15m 7s trunk passed +1 compile 23m 26s trunk passed +1 checkstyle 2m 16s trunk passed +1 mvnsite 2m 0s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 3m 35s trunk passed +1 javadoc 1m 34s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 14s the patch passed +1 compile 16m 54s the patch passed -1 javac 16m 54s root generated 1 new + 777 unchanged - 1 fixed = 778 total (was 778) +1 checkstyle 2m 3s the patch passed +1 mvnsite 1m 46s the patch passed +1 mvneclipse 0m 44s the patch passed -1 whitespace 0m 0s The patch has 9 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 3m 28s the patch passed +1 javadoc 1m 27s the patch passed -1 unit 8m 5s hadoop-common in the patch failed. +1 unit 1m 8s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 40s The patch does not generate ASF License warnings. 111m 27s Reason Tests Failed junit tests hadoop.security.TestKDiag   hadoop.fs.viewfs.TestViewFsWithAuthorityLocalFs   hadoop.fs.viewfs.TestViewFsLocalFs Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-9631 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12859959/HADOOP-9631.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 534a3124848e 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f462e1f Default Java 1.8.0_121 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/artifact/patchprocess/diff-compile-javac-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11879/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xkrogen Erik Krogen added a comment -

          Resuming this work; attaching a v005 patch incorporating Sanjay Radia's suggestions. Added a unit test to verify.

          Show
          xkrogen Erik Krogen added a comment - Resuming this work; attaching a v005 patch incorporating Sanjay Radia 's suggestions. Added a unit test to verify.
          Hide
          q79969786 Yuming Wang added a comment -

          Lohit Vijayarenu, Can you please help me upload the latest patch, I'm not authorized.

          Show
          q79969786 Yuming Wang added a comment - Lohit Vijayarenu , Can you please help me upload the latest patch , I'm not authorized.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 5s HADOOP-9631 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12588630/HADOOP-9631.trunk.4.patch
          JIRA Issue HADOOP-9631
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9920/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 0s Docker mode activated. -1 patch 0m 5s HADOOP-9631 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12588630/HADOOP-9631.trunk.4.patch JIRA Issue HADOOP-9631 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9920/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          q79969786 Yuming Wang added a comment -

          The test failures still exist?

          Show
          q79969786 Yuming Wang added a comment - The test failures still exist?
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 0s The patch command could not apply the patch during dryrun.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12588630/HADOOP-9631.trunk.4.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / f1a152c
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6316/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 0s The patch command could not apply the patch during dryrun. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12588630/HADOOP-9631.trunk.4.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / f1a152c Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6316/console This message was automatically generated.
          Hide
          sanjay.radia Sanjay Radia added a comment -

          Here are two early comments (haven't finished reviewing the whole patch).

          • viewfs#getServerDefaults(path) can be simplified. See how open or list are implemented and it take advantage of the internal class InternalDirOfViewOf.
            Something like this should work:
            viewfs#getServerDefaults(f) {
              InodeTree.ResolveResult<AbstractFileSystem> res = 
                    fsState.resolve(getUriPath(f), true);
                return res.targetFileSystem.getServerDefailts(res.remainingPath);
            }
            
            
            InternalDirOfViewFs#getServerDefaults() {
            return LocalConfigKeys.getServerDefaults();
            }
            InternalDirOfViewFs#getServerDefaults(f) {
                checkPathIsSlash(f);
            return LocalConfigKeys.getServerDefaults();
            }
            
          • FIleSystem#getServerDefaults(f) is incorrect due to getDefaultReplication(). It should use getDefaultReplciation(f). Hence move the code for FIleSystem#getServerDefaults() to FIleSystem#getServerDefaults(f), changing the getDefaultReplication to pass the pathname f. Have FIleSystem#getServerDefaults() call FIleSystem#getServerDefaults("/");
          Show
          sanjay.radia Sanjay Radia added a comment - Here are two early comments (haven't finished reviewing the whole patch). viewfs#getServerDefaults(path) can be simplified. See how open or list are implemented and it take advantage of the internal class InternalDirOfViewOf. Something like this should work: viewfs#getServerDefaults(f) { InodeTree.ResolveResult<AbstractFileSystem> res = fsState.resolve(getUriPath(f), true ); return res.targetFileSystem.getServerDefailts(res.remainingPath); } InternalDirOfViewFs#getServerDefaults() { return LocalConfigKeys.getServerDefaults(); } InternalDirOfViewFs#getServerDefaults(f) { checkPathIsSlash(f); return LocalConfigKeys.getServerDefaults(); } FIleSystem#getServerDefaults(f) is incorrect due to getDefaultReplication(). It should use getDefaultReplciation(f). Hence move the code for FIleSystem#getServerDefaults() to FIleSystem#getServerDefaults(f), changing the getDefaultReplication to pass the pathname f. Have FIleSystem#getServerDefaults() call FIleSystem#getServerDefaults("/");
          Hide
          lohit Lohit Vijayarenu added a comment -

          Chris Nauroth Can you please help review latest patch

          Show
          lohit Lohit Vijayarenu added a comment - Chris Nauroth Can you please help review latest patch
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12588630/HADOOP-9631.trunk.4.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 2 new or modified test files.

          +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-HADOOP-Build/2675//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2675//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12588630/HADOOP-9631.trunk.4.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +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-HADOOP-Build/2675//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2675//console This message is automatically generated.
          Hide
          lohit Lohit Vijayarenu added a comment -

          Patch to fix javadoc warning. org.apache.hadoop.hdfs.server.blockmanagement.TestBlocksWithNotEnoughRacks failure does not look to be related to this patch. I see this failure for other patches too.

          Show
          lohit Lohit Vijayarenu added a comment - Patch to fix javadoc warning. org.apache.hadoop.hdfs.server.blockmanagement.TestBlocksWithNotEnoughRacks failure does not look to be related to this patch. I see this failure for other patches too.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12588487/HADOOP-9631.trunk.3.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 3 new or modified test files.

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

          -1 javadoc. The javadoc tool appears to have generated 1 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.blockmanagement.TestBlocksWithNotEnoughRacks

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

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12588487/HADOOP-9631.trunk.3.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 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.blockmanagement.TestBlocksWithNotEnoughRacks +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2670//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2670//console This message is automatically generated.
          Hide
          lohit Lohit Vijayarenu added a comment -

          Another try at this patch. Problem I was facing was passing down getServerDefaults() call from InternalViewOfDir class. So, in ViewFs::getServerDeaults() did a match of mountpoints and if there exists a matching mountpoint return the filesystem, else return LocalConfig as earlier. This was needed so that we can handle case of FileNotFound, invalid dir/file creation and such.

          Show
          lohit Lohit Vijayarenu added a comment - Another try at this patch. Problem I was facing was passing down getServerDefaults() call from InternalViewOfDir class. So, in ViewFs::getServerDeaults() did a match of mountpoints and if there exists a matching mountpoint return the filesystem, else return LocalConfig as earlier. This was needed so that we can handle case of FileNotFound, invalid dir/file creation and such.
          Hide
          lohit Lohit Vijayarenu added a comment -

          Taking a look at how ViewFs mountable is constructed, it looks there is no straightforward way to get TargetFileSystem given mount point or path. I started with changing getServerDefaults to accept Path. Now, create call would pass a Path which does not exists. So, we will have to resolve the Path to mountpoint. I see that ViewFileSystem::resolve can fetch the mountpoint, but not an easy way to fetch underlying FileSystem. '/' seem to always resolve to InteralViewOfDir which does not have targetFileSytem. Any suggestions of easy way to fetch this?

          Show
          lohit Lohit Vijayarenu added a comment - Taking a look at how ViewFs mountable is constructed, it looks there is no straightforward way to get TargetFileSystem given mount point or path. I started with changing getServerDefaults to accept Path. Now, create call would pass a Path which does not exists. So, we will have to resolve the Path to mountpoint. I see that ViewFileSystem::resolve can fetch the mountpoint, but not an easy way to fetch underlying FileSystem. '/' seem to always resolve to InteralViewOfDir which does not have targetFileSytem. Any suggestions of easy way to fetch this?
          Hide
          lohit Lohit Vijayarenu added a comment -

          Chris Nauroth Thanks for the review. I see the problem where viewfs might be mapped to different filesystems. Let me revisit my initial patch which was trying to use Path to resolve to FileSystem mountpoint and update JIRA.

          Show
          lohit Lohit Vijayarenu added a comment - Chris Nauroth Thanks for the review. I see the problem where viewfs might be mapped to different filesystems. Let me revisit my initial patch which was trying to use Path to resolve to FileSystem mountpoint and update JIRA.
          Hide
          cnauroth Chris Nauroth added a comment -

          Hi Lohit,

          I'm looking at version 2 of the patch, which always resolves to the home directory to get server defaults. This might appear to be working for the specific use case you need (YARN NodeManager log aggregation), but I don't think it would be generally correct in all cases.

          For example, imagine a viewfs configured with mount points mapping /user to local file system and /data to an HDFS cluster. Then, creating a file in /data would still get the undesired replication value.

          That example is a bit contrived, but we can also consider the more realistic example from the ViewFs javadocs:

           * For example one could have a mount table that provides links such as
           * <ul>
           * <li>  /user          -> hdfs://nnContainingUserDir/user
           * <li>  /project/foo   -> hdfs://nnProject1/projects/foo
           * <li>  /project/bar   -> hdfs://nnProject2/projects/bar
           * <li>  /tmp           -> hdfs://nnTmp/privateTmpForUserXXX
           * </ul> 
          

          Now, assume the client is creating file /project/foo/myFile. The operation would pick up server defaults associated with /user (configured on nnContainingUserDir) instead of the desired /project/foo (configured on nnProject1). The server defaults may differ between nnContainingUserDir and nnProject1.

          It seems like we need a patch that is sensitive to the actual Path accessed in the client operation, similar to what Daryn mentioned earlier.

          Also, I don't think the new unit test exercises this code, because TestHDFSFileContextMainOperations doesn't configure a ViewFs.

          Show
          cnauroth Chris Nauroth added a comment - Hi Lohit, I'm looking at version 2 of the patch, which always resolves to the home directory to get server defaults. This might appear to be working for the specific use case you need (YARN NodeManager log aggregation), but I don't think it would be generally correct in all cases. For example, imagine a viewfs configured with mount points mapping /user to local file system and /data to an HDFS cluster. Then, creating a file in /data would still get the undesired replication value. That example is a bit contrived, but we can also consider the more realistic example from the ViewFs javadocs: * For example one could have a mount table that provides links such as * <ul> * <li> /user -> hdfs: //nnContainingUserDir/user * <li> /project/foo -> hdfs: //nnProject1/projects/foo * <li> /project/bar -> hdfs: //nnProject2/projects/bar * <li> /tmp -> hdfs: //nnTmp/privateTmpForUserXXX * </ul> Now, assume the client is creating file /project/foo/myFile. The operation would pick up server defaults associated with /user (configured on nnContainingUserDir) instead of the desired /project/foo (configured on nnProject1). The server defaults may differ between nnContainingUserDir and nnProject1. It seems like we need a patch that is sensitive to the actual Path accessed in the client operation, similar to what Daryn mentioned earlier. Also, I don't think the new unit test exercises this code, because TestHDFSFileContextMainOperations doesn't configure a ViewFs .
          Hide
          lohit Lohit Vijayarenu added a comment -

          Can anyone please review this.

          Show
          lohit Lohit Vijayarenu added a comment - Can anyone please review this.
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12587367/HADOOP-9631.trunk.2.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 1 new or modified test files.

          +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-HADOOP-Build/2639//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2639//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12587367/HADOOP-9631.trunk.2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +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-HADOOP-Build/2639//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2639//console This message is automatically generated.
          Hide
          lohit Lohit Vijayarenu added a comment -

          While trying to see why testcases failed, realized that there is easy way to this. In the end each filesystem has ServerDefaults, so in viewfs we just had to pick right filesystem and pass it down. Made a change to use homedirectory path and chose underlying filesystem. Attaching new patch with this change. Now all viewfs tests as well as new test works.

          Show
          lohit Lohit Vijayarenu added a comment - While trying to see why testcases failed, realized that there is easy way to this. In the end each filesystem has ServerDefaults, so in viewfs we just had to pick right filesystem and pass it down. Made a change to use homedirectory path and chose underlying filesystem. Attaching new patch with this change. Now all viewfs tests as well as new test works.
          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 new or modified test files.

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

          -1 javadoc. The javadoc tool appears to have generated 1 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.fs.viewfs.TestViewFsWithAuthorityLocalFs
          org.apache.hadoop.fs.viewfs.TestViewFsLocalFs
          org.apache.hadoop.fs.viewfs.TestViewFsHdfs
          org.apache.hadoop.fs.viewfs.TestViewFsAtHdfsRoot

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

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12587250/HADOOP-9631.trunk.1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 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.fs.viewfs.TestViewFsWithAuthorityLocalFs org.apache.hadoop.fs.viewfs.TestViewFsLocalFs org.apache.hadoop.fs.viewfs.TestViewFsHdfs org.apache.hadoop.fs.viewfs.TestViewFsAtHdfsRoot +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2632//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2632//console This message is automatically generated.
          Hide
          lohit Lohit Vijayarenu added a comment -

          Attached is patch which deprecates getServerDefaults() and adds getServerDefaults(Path). Tested this by deploying on one of our YARN cluster and could see App logs getting created with replication factor of 3 instead of 1.

          Show
          lohit Lohit Vijayarenu added a comment - Attached is patch which deprecates getServerDefaults() and adds getServerDefaults(Path). Tested this by deploying on one of our YARN cluster and could see App logs getting created with replication factor of 3 instead of 1.
          Hide
          daryn Daryn Sharp added a comment -

          A similar change as made to the FileSystem api in HADOOP-8014 is needed.

          Show
          daryn Daryn Sharp added a comment - A similar change as made to the FileSystem api in HADOOP-8014 is needed.
          Hide
          lohit Lohit Vijayarenu added a comment -

          We see a case where NodeManager Log Aggregator service uses FileContext to move container log from local filesystem to HDFS. If default filesystem on HDFS cluster is set to viewfs:/// then FileContext would internally end up using ViewFs. While doing so, any files created via FileContext would be created with replication factor of 1. This is because ViewFs getServerDefaults seems to return local fileystem defaults

          @Override
            public FsServerDefaults getServerDefaults() throws IOException {
              return LocalConfigKeys.getServerDefaults(); 
            }
          

          This would become problem for anyone using new FileContext API running on top of a federated namespace.
          Note that this is not problem with FileSystem APIs. Attached is a test program which creates file using both APIs on HDFS and doing ls on that file shows both have different replication factor. File created using FileContext has replication factor of 1, while file created using FileSystem has replication factor of 3.

          Show
          lohit Lohit Vijayarenu added a comment - We see a case where NodeManager Log Aggregator service uses FileContext to move container log from local filesystem to HDFS. If default filesystem on HDFS cluster is set to viewfs:/// then FileContext would internally end up using ViewFs. While doing so, any files created via FileContext would be created with replication factor of 1. This is because ViewFs getServerDefaults seems to return local fileystem defaults @Override public FsServerDefaults getServerDefaults() throws IOException { return LocalConfigKeys.getServerDefaults(); } This would become problem for anyone using new FileContext API running on top of a federated namespace. Note that this is not problem with FileSystem APIs. Attached is a test program which creates file using both APIs on HDFS and doing ls on that file shows both have different replication factor. File created using FileContext has replication factor of 1, while file created using FileSystem has replication factor of 3.

            People

            • Assignee:
              xkrogen Erik Krogen
              Reporter:
              lohit Lohit Vijayarenu
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development