Details

      Description

      Quoting my comments in HDFS-10974:

      Looking at code again, I think we can improve the handling of EC files better:

      • In SetReplication#processPath, can we check if the file is EC before invoking setReplication() on it?
      • -setrep also has a command switch -w to wait for the file to be replicated. Can we ignore this if the file is erasure coded?

      Also, in fact, -setrep only make sense for HDFS file system...

      1. HDFS-11598.001.patch
        4 kB
        Yiqun Lin
      2. HDFS-11598.002.patch
        4 kB
        Yiqun Lin

        Activity

        Hide
        linyiqun Yiqun Lin added a comment -

        Thanks for catching this, Wei-Chiu Chuang!
        I take some time to make this improvement. It's a minor change but it should be nice to have in hdfs ec.
        Attach my patch. Kindly review. Thanks.

        Show
        linyiqun Yiqun Lin added a comment - Thanks for catching this, Wei-Chiu Chuang ! I take some time to make this improvement. It's a minor change but it should be nice to have in hdfs ec. Attach my patch. Kindly review. Thanks.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 23s 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 1m 51s Maven dependency ordering for branch
        +1 mvninstall 12m 44s trunk passed
        +1 compile 16m 14s trunk passed
        +1 checkstyle 1m 57s trunk passed
        +1 mvnsite 2m 21s trunk passed
        +1 mvneclipse 0m 34s trunk passed
        +1 findbugs 3m 49s trunk passed
        +1 javadoc 1m 40s trunk passed
        0 mvndep 0m 16s Maven dependency ordering for patch
        +1 mvninstall 1m 46s the patch passed
        +1 compile 16m 17s the patch passed
        +1 javac 16m 17s the patch passed
        +1 checkstyle 2m 1s root: The patch generated 0 new + 17 unchanged - 1 fixed = 17 total (was 18)
        +1 mvnsite 2m 18s the patch passed
        +1 mvneclipse 0m 35s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 55s the patch passed
        +1 javadoc 1m 35s the patch passed
        -1 unit 7m 47s hadoop-common in the patch failed.
        +1 unit 63m 39s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 36s The patch does not generate ASF License warnings.
        143m 50s



        Reason Tests
        Failed junit tests hadoop.security.TestKDiag



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11598
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861422/HDFS-11598.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 61d4995d9c0f 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 / 28cdc5a
        Default Java 1.8.0_121
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/18932/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18932/testReport/
        modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18932/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 23s 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 1m 51s Maven dependency ordering for branch +1 mvninstall 12m 44s trunk passed +1 compile 16m 14s trunk passed +1 checkstyle 1m 57s trunk passed +1 mvnsite 2m 21s trunk passed +1 mvneclipse 0m 34s trunk passed +1 findbugs 3m 49s trunk passed +1 javadoc 1m 40s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 46s the patch passed +1 compile 16m 17s the patch passed +1 javac 16m 17s the patch passed +1 checkstyle 2m 1s root: The patch generated 0 new + 17 unchanged - 1 fixed = 17 total (was 18) +1 mvnsite 2m 18s the patch passed +1 mvneclipse 0m 35s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 55s the patch passed +1 javadoc 1m 35s the patch passed -1 unit 7m 47s hadoop-common in the patch failed. +1 unit 63m 39s hadoop-hdfs in the patch passed. +1 asflicense 0m 36s The patch does not generate ASF License warnings. 143m 50s Reason Tests Failed junit tests hadoop.security.TestKDiag Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11598 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861422/HDFS-11598.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 61d4995d9c0f 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 / 28cdc5a Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/18932/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18932/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18932/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        Hello Yiqun Lin thanks for your quick patch!

        This is probably my oversight initially....
        If we do not add an erasure coded file into waitList, we do not need to check if a file is erasure coded in waitForReplication(), correct?

        For the error message "Do not set replication for: ", it reads more naturally to use past tense "Did not set replication for:"

        Also in the test, would you mind to add an additional assert to make sure the replication factor of the EC file is still 1?

        Show
        jojochuang Wei-Chiu Chuang added a comment - Hello Yiqun Lin thanks for your quick patch! This is probably my oversight initially.... If we do not add an erasure coded file into waitList, we do not need to check if a file is erasure coded in waitForReplication(), correct? For the error message "Do not set replication for: ", it reads more naturally to use past tense "Did not set replication for:" Also in the test, would you mind to add an additional assert to make sure the replication factor of the EC file is still 1?
        Hide
        linyiqun Yiqun Lin added a comment -

        Thanks Wei-Chiu Chuang for the review!

        If we do not add an erasure coded file into waitList, we do not need to check if a file is erasure coded in waitForReplication(), correct?

        Yes, you are right. The other two comments make sense to me.
        Attach the new patch.

        Show
        linyiqun Yiqun Lin added a comment - Thanks Wei-Chiu Chuang for the review! If we do not add an erasure coded file into waitList, we do not need to check if a file is erasure coded in waitForReplication(), correct? Yes, you are right. The other two comments make sense to me. Attach the new patch.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 20s 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 2m 1s Maven dependency ordering for branch
        +1 mvninstall 13m 5s trunk passed
        +1 compile 14m 55s trunk passed
        +1 checkstyle 1m 54s trunk passed
        +1 mvnsite 2m 3s trunk passed
        +1 mvneclipse 0m 39s trunk passed
        +1 findbugs 3m 16s trunk passed
        +1 javadoc 1m 35s trunk passed
        0 mvndep 0m 14s Maven dependency ordering for patch
        +1 mvninstall 1m 26s the patch passed
        +1 compile 13m 33s the patch passed
        +1 javac 13m 33s the patch passed
        +1 checkstyle 1m 55s root: The patch generated 0 new + 17 unchanged - 1 fixed = 17 total (was 18)
        +1 mvnsite 1m 59s the patch passed
        +1 mvneclipse 0m 38s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 31s the patch passed
        +1 javadoc 1m 36s the patch passed
        +1 unit 8m 18s hadoop-common in the patch passed.
        -1 unit 64m 28s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 36s The patch does not generate ASF License warnings.
        139m 23s



        Reason Tests
        Failed junit tests hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11598
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861698/HDFS-11598.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 4a52c528efa4 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 / 845529b
        Default Java 1.8.0_121
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/18954/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18954/testReport/
        modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18954/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 20s 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 2m 1s Maven dependency ordering for branch +1 mvninstall 13m 5s trunk passed +1 compile 14m 55s trunk passed +1 checkstyle 1m 54s trunk passed +1 mvnsite 2m 3s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 3m 16s trunk passed +1 javadoc 1m 35s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 13m 33s the patch passed +1 javac 13m 33s the patch passed +1 checkstyle 1m 55s root: The patch generated 0 new + 17 unchanged - 1 fixed = 17 total (was 18) +1 mvnsite 1m 59s the patch passed +1 mvneclipse 0m 38s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 31s the patch passed +1 javadoc 1m 36s the patch passed +1 unit 8m 18s hadoop-common in the patch passed. -1 unit 64m 28s hadoop-hdfs in the patch failed. +1 asflicense 0m 36s The patch does not generate ASF License warnings. 139m 23s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11598 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861698/HDFS-11598.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4a52c528efa4 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 / 845529b Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/18954/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18954/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18954/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        +1

        Show
        jojochuang Wei-Chiu Chuang added a comment - +1
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        Thanks a lot for the patch Yiqun Lin. I committed your 002 patch to trunk.

        Show
        jojochuang Wei-Chiu Chuang added a comment - Thanks a lot for the patch Yiqun Lin . I committed your 002 patch to trunk.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11517 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11517/)
        HDFS-11598. Improve -setrep for Erasure Coded files. Contributed by (weichiu: rev bbd68478d5743b3b2911bf3febed7daa89479e45)

        • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SetReplication.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSetrepIncreasing.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11517 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11517/ ) HDFS-11598 . Improve -setrep for Erasure Coded files. Contributed by (weichiu: rev bbd68478d5743b3b2911bf3febed7daa89479e45) (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SetReplication.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSetrepIncreasing.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11591 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11591/)
        HDFS-11598. Improve -setrep for Erasure Coded files. Contributed by (weichiu: rev bbd68478d5743b3b2911bf3febed7daa89479e45)

        • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SetReplication.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSetrepIncreasing.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11591 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11591/ ) HDFS-11598 . Improve -setrep for Erasure Coded files. Contributed by (weichiu: rev bbd68478d5743b3b2911bf3febed7daa89479e45) (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SetReplication.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSetrepIncreasing.java

          People

          • Assignee:
            linyiqun Yiqun Lin
            Reporter:
            jojochuang Wei-Chiu Chuang
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development