Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-12395

Support erasure coding policy operations in namenode edit log

    Details

    • Target Version/s:
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      See RN in HDFS-7859 as part of the work.

      Description

      Support add, remove, disable, enable erasure coding policy operation in edit log.

      1. editsStored
        6 kB
        SammiChen
      2. HDFS-12395.001.patch
        39 kB
        SammiChen
      3. HDFS-12395.002.patch
        50 kB
        SammiChen
      4. HDFS-12395.003.patch
        51 kB
        SammiChen
      5. HDFS-12395.004.patch
        50 kB
        SammiChen

        Issue Links

          Activity

          Hide
          Sammi SammiChen added a comment -

          Thanks Kihwal Lee for the reminder of taking care the NN layout version, I have the same opinion as Andrew Wang.

          And Brahma Reddy Battula, thanks for your advice. I will issue separate JIRA next time in this case.

          Show
          Sammi SammiChen added a comment - Thanks Kihwal Lee for the reminder of taking care the NN layout version, I have the same opinion as Andrew Wang . And Brahma Reddy Battula , thanks for your advice. I will issue separate JIRA next time in this case.
          Hide
          kihwal Kihwal Lee added a comment -

          since we don't need to support upgrade from alpha->beta

          In that case, it should be fine. I was thinking it needs to be bumped up since we already had a release with EC.

          Show
          kihwal Kihwal Lee added a comment - since we don't need to support upgrade from alpha->beta In that case, it should be fine. I was thinking it needs to be bumped up since we already had a release with EC.
          Hide
          andrew.wang Andrew Wang added a comment - - edited

          Kihwal Lee EC added LV -64 when the branch was merged. Since we've been in alpha up to now, the thinking was that we can piggyback on the existing EC layout version since we don't need to support upgrade from alpha->beta.

          Show
          andrew.wang Andrew Wang added a comment - - edited Kihwal Lee EC added LV -64 when the branch was merged. Since we've been in alpha up to now, the thinking was that we can piggyback on the existing EC layout version since we don't need to support upgrade from alpha->beta.
          Hide
          kihwal Kihwal Lee added a comment -

          You need to increment the NN layout version.

          Show
          kihwal Kihwal Lee added a comment - You need to increment the NN layout version.
          Hide
          brahmareddy Brahma Reddy Battula added a comment -

          Thanks for taking care, could have fixed the testcases in seperate jira since HDFS-12460 states "Make addErasureCodingPolicy an idempotent operation". Anyway it's committed,should be fine I feel.

          Show
          brahmareddy Brahma Reddy Battula added a comment - Thanks for taking care, could have fixed the testcases in seperate jira since HDFS-12460 states "Make addErasureCodingPolicy an idempotent operation". Anyway it's committed,should be fine I feel.
          Hide
          drankye Kai Zheng added a comment -

          Thanks Brahma for raising this. I just got HDFS-12460 in. SammiChen please help double check with the latest trunk, if the two failures are fixed or not. Thanks.

          Show
          drankye Kai Zheng added a comment - Thanks Brahma for raising this. I just got HDFS-12460 in. SammiChen please help double check with the latest trunk, if the two failures are fixed or not. Thanks.
          Hide
          Sammi SammiChen added a comment -

          Hi, Brahma Reddy Battula, thanks for the reminder. I will address these two failed cases in HDFS-12460.

          Show
          Sammi SammiChen added a comment - Hi, Brahma Reddy Battula , thanks for the reminder. I will address these two failed cases in HDFS-12460 .
          Hide
          brahmareddy Brahma Reddy Battula added a comment -

          Following two test fails after this commit.

          TestNamenodeRetryCache.testRetryCacheRebuild
          TestRetryCacheWithHA.testRetryCacheOnStandbyNN

          Reference:

          https://builds.apache.org/job/PreCommit-HDFS-Build/21189/testReport/

          Trace
          java.lang.AssertionError: Retry cache size is wrong expected:<26> but was:<34>
          at org.junit.Assert.fail(Assert.java:88)
          at org.junit.Assert.failNotEquals(Assert.java:743)
          at org.junit.Assert.assertEquals(Assert.java:118)
          at org.junit.Assert.assertEquals(Assert.java:555)
          at org.apache.hadoop.hdfs.server.namenode.TestNamenodeRetryCache.testRetryCacheRebuild(TestNamenodeRetryCache.java:439)

          Show
          brahmareddy Brahma Reddy Battula added a comment - Following two test fails after this commit. TestNamenodeRetryCache.testRetryCacheRebuild TestRetryCacheWithHA.testRetryCacheOnStandbyNN Reference: https://builds.apache.org/job/PreCommit-HDFS-Build/21189/testReport/ Trace java.lang.AssertionError: Retry cache size is wrong expected:<26> but was:<34> at org.junit.Assert.fail(Assert.java:88) at org.junit.Assert.failNotEquals(Assert.java:743) at org.junit.Assert.assertEquals(Assert.java:118) at org.junit.Assert.assertEquals(Assert.java:555) at org.apache.hadoop.hdfs.server.namenode.TestNamenodeRetryCache.testRetryCacheRebuild(TestNamenodeRetryCache.java:439)
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12879 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12879/)
          HDFS-12395. Support erasure coding policy operations in namenode edit (kai.zheng: rev 08d996d3e9265efad737efad50cbc5b10a0202f8)

          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSEditLogLoader.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSInotifyEventInputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOpCodes.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HdfsEditsViewer.md
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirErasureCodingOp.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/TestOfflineEditsViewer.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageSerialization.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12879 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12879/ ) HDFS-12395 . Support erasure coding policy operations in namenode edit (kai.zheng: rev 08d996d3e9265efad737efad50cbc5b10a0202f8) (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSEditLogLoader.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSInotifyEventInputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOpCodes.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HdfsEditsViewer.md (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirErasureCodingOp.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/TestOfflineEditsViewer.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageSerialization.java
          Hide
          drankye Kai Zheng added a comment -

          Committed to branch-3.0 and trunk. Thanks SammiChen for the contribution, Rakesh R and Lei (Eddy) Xu for the review!

          Show
          drankye Kai Zheng added a comment - Committed to branch-3.0 and trunk. Thanks SammiChen for the contribution, Rakesh R and Lei (Eddy) Xu for the review!
          Hide
          drankye Kai Zheng added a comment -

          Checked the building results and it looks good. The relevant failure TestOfflineEditsViewer.testStored will be fixed after the check in, as commented by Sammi above. The minor check style will be addressed before check in.

          Will commit it shortly.

          Show
          drankye Kai Zheng added a comment - Checked the building results and it looks good. The relevant failure TestOfflineEditsViewer.testStored will be fixed after the check in, as commented by Sammi above. The minor check style will be addressed before check in. Will commit it shortly.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 13m 50s trunk passed
          +1 compile 0m 50s trunk passed
          +1 checkstyle 0m 49s trunk passed
          +1 mvnsite 0m 56s trunk passed
          +1 findbugs 1m 51s trunk passed
          +1 javadoc 0m 43s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 52s the patch passed
          +1 compile 0m 49s the patch passed
          +1 javac 0m 49s the patch passed
          -0 checkstyle 0m 48s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 1020 unchanged - 0 fixed = 1021 total (was 1020)
          +1 mvnsite 0m 53s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 1m 57s the patch passed
          +1 javadoc 0m 40s the patch passed
                Other Tests
          -1 unit 93m 32s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          120m 29s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA
            hadoop.hdfs.TestErasureCodingPolicies
            hadoop.hdfs.TestFileAppendRestart
            hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
            hadoop.hdfs.server.namenode.TestDecommissioningStatus
            hadoop.hdfs.qjournal.server.TestJournalNodeSync
            hadoop.hdfs.server.namenode.metrics.TestNameNodeMetrics
            hadoop.hdfs.TestClientProtocolForPipelineRecovery
            hadoop.hdfs.TestLeaseRecoveryStriped
            hadoop.hdfs.TestErasureCodingPoliciesWithRandomECPolicy
            hadoop.hdfs.TestEncryptedTransfer
            hadoop.hdfs.server.namenode.TestNamenodeRetryCache
          Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue HDFS-12395
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887039/HDFS-12395.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 74c4b68d3bc6 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 390c2b5
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/21148/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/21148/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/21148/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/21148/console
          Powered by Apache Yetus 0.6.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.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.       trunk Compile Tests +1 mvninstall 13m 50s trunk passed +1 compile 0m 50s trunk passed +1 checkstyle 0m 49s trunk passed +1 mvnsite 0m 56s trunk passed +1 findbugs 1m 51s trunk passed +1 javadoc 0m 43s trunk passed       Patch Compile Tests +1 mvninstall 0m 52s the patch passed +1 compile 0m 49s the patch passed +1 javac 0m 49s the patch passed -0 checkstyle 0m 48s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 1020 unchanged - 0 fixed = 1021 total (was 1020) +1 mvnsite 0m 53s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 57s the patch passed +1 javadoc 0m 40s the patch passed       Other Tests -1 unit 93m 32s hadoop-hdfs in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 120m 29s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA   hadoop.hdfs.TestErasureCodingPolicies   hadoop.hdfs.TestFileAppendRestart   hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer   hadoop.hdfs.server.namenode.TestDecommissioningStatus   hadoop.hdfs.qjournal.server.TestJournalNodeSync   hadoop.hdfs.server.namenode.metrics.TestNameNodeMetrics   hadoop.hdfs.TestClientProtocolForPipelineRecovery   hadoop.hdfs.TestLeaseRecoveryStriped   hadoop.hdfs.TestErasureCodingPoliciesWithRandomECPolicy   hadoop.hdfs.TestEncryptedTransfer   hadoop.hdfs.server.namenode.TestNamenodeRetryCache Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue HDFS-12395 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887039/HDFS-12395.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 74c4b68d3bc6 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 390c2b5 Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/21148/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/21148/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/21148/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/21148/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          drankye Kai Zheng added a comment -

          Thanks Sammi for the update. The latest patch LGTM. +1 pending on the building.

          Show
          drankye Kai Zheng added a comment - Thanks Sammi for the update. The latest patch LGTM. +1 pending on the building.
          Hide
          Sammi SammiChen added a comment -

          1. fix style issues
          2. uncomment the piece of code

          Another note is the uploaded "editsStored" file is also a part of the patch. It's a binary file. After apply the HDFS-12395.004.patch to the trunk, need to replace the "editsStored" file under hadoop/hadoop-hdfs-project/hadoop-hdfs/src/test/resources with the uploaded one.

          Otherwise one unit test will failure, just as the build system reports

          org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer.testStored
          Error Details

          Reference XML edits and parsed to XML should be same
          Stack Trace
          Standard Output

          Show
          Sammi SammiChen added a comment - 1. fix style issues 2. uncomment the piece of code Another note is the uploaded "editsStored" file is also a part of the patch. It's a binary file. After apply the HDFS-12395 .004.patch to the trunk, need to replace the "editsStored" file under hadoop/hadoop-hdfs-project/hadoop-hdfs/src/test/resources with the uploaded one. Otherwise one unit test will failure, just as the build system reports org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer.testStored Error Details Reference XML edits and parsed to XML should be same Stack Trace Standard Output
          Hide
          Sammi SammiChen added a comment - - edited

          Thanks Lei (Eddy) Xu and Kai Zheng! OEV is already been implemented in this JIRA since several unit test will failure without it. Th code is commented out during debug. Forget to uncomment it . Will make it correct in next patch.

          Show
          Sammi SammiChen added a comment - - edited Thanks Lei (Eddy) Xu and Kai Zheng ! OEV is already been implemented in this JIRA since several unit test will failure without it. Th code is commented out during debug. Forget to uncomment it . Will make it correct in next patch.
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          LGTM.

          	//    assertTrue("Edits " + editsStored + " should have all op codes",
                  //        hasAllOpCodes(editsStored));
          

          Is this going to be uncommented after OEV being implemented?

          +1 pending.

          Show
          eddyxu Lei (Eddy) Xu added a comment - LGTM. // assertTrue( "Edits " + editsStored + " should have all op codes" , // hasAllOpCodes(editsStored)); Is this going to be uncommented after OEV being implemented? +1 pending.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 17m 37s trunk passed
          +1 compile 1m 5s trunk passed
          +1 checkstyle 1m 2s trunk passed
          +1 mvnsite 1m 17s trunk passed
          +1 findbugs 2m 18s trunk passed
          +1 javadoc 0m 49s trunk passed
                Patch Compile Tests
          +1 mvninstall 1m 8s the patch passed
          +1 compile 1m 5s the patch passed
          +1 javac 1m 5s the patch passed
          -0 checkstyle 0m 55s hadoop-hdfs-project/hadoop-hdfs: The patch generated 7 new + 1018 unchanged - 0 fixed = 1025 total (was 1018)
          +1 mvnsite 1m 8s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 2s The patch has no ill-formed XML file.
          +1 findbugs 1m 57s the patch passed
          +1 javadoc 0m 40s the patch passed
                Other Tests
          -1 unit 88m 36s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          121m 52s



          Reason Tests
          Failed junit tests hadoop.hdfs.qjournal.server.TestJournalNodeSync
            hadoop.hdfs.TestAbandonBlock
            hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA
            hadoop.hdfs.server.namenode.TestReencryptionWithKMS
            hadoop.hdfs.server.namenode.TestNamenodeRetryCache
            hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
            hadoop.hdfs.TestLeaseRecoveryStriped
            hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks
            hadoop.hdfs.TestBlockStoragePolicy
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
          Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue HDFS-12395
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12886845/HDFS-12395.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 1de1bf7fc0fa 3.13.0-123-generic #172-Ubuntu SMP Mon Jun 26 18:04:35 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / fa6cc43
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/21115/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/21115/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/21115/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/21115/console
          Powered by Apache Yetus 0.6.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.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.       trunk Compile Tests +1 mvninstall 17m 37s trunk passed +1 compile 1m 5s trunk passed +1 checkstyle 1m 2s trunk passed +1 mvnsite 1m 17s trunk passed +1 findbugs 2m 18s trunk passed +1 javadoc 0m 49s trunk passed       Patch Compile Tests +1 mvninstall 1m 8s the patch passed +1 compile 1m 5s the patch passed +1 javac 1m 5s the patch passed -0 checkstyle 0m 55s hadoop-hdfs-project/hadoop-hdfs: The patch generated 7 new + 1018 unchanged - 0 fixed = 1025 total (was 1018) +1 mvnsite 1m 8s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 1m 57s the patch passed +1 javadoc 0m 40s the patch passed       Other Tests -1 unit 88m 36s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 121m 52s Reason Tests Failed junit tests hadoop.hdfs.qjournal.server.TestJournalNodeSync   hadoop.hdfs.TestAbandonBlock   hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA   hadoop.hdfs.server.namenode.TestReencryptionWithKMS   hadoop.hdfs.server.namenode.TestNamenodeRetryCache   hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer   hadoop.hdfs.TestLeaseRecoveryStriped   hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks   hadoop.hdfs.TestBlockStoragePolicy   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue HDFS-12395 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12886845/HDFS-12395.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 1de1bf7fc0fa 3.13.0-123-generic #172-Ubuntu SMP Mon Jun 26 18:04:35 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fa6cc43 Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/21115/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/21115/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/21115/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/21115/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          drankye Kai Zheng added a comment -

          Lei (Eddy) Xu, would you also take a look at this? Thanks. If no few more comments I'll get this in tomorrow.

          Show
          drankye Kai Zheng added a comment - Lei (Eddy) Xu , would you also take a look at this? Thanks. If no few more comments I'll get this in tomorrow.
          Hide
          drankye Kai Zheng added a comment -

          The latest patch looks pretty good and close. Only a minor:
          Is this intended?

          -    assertTrue("Edits " + editsStored + " should have all op codes",
          -        hasAllOpCodes(editsStored));
          +//    assertTrue("Edits " + editsStored + " should have all op codes",
          +//        hasAllOpCodes(editsStored));
          

          +1 pending on the fix.

          Show
          drankye Kai Zheng added a comment - The latest patch looks pretty good and close. Only a minor: Is this intended? - assertTrue( "Edits " + editsStored + " should have all op codes" , - hasAllOpCodes(editsStored)); + // assertTrue( "Edits " + editsStored + " should have all op codes" , + // hasAllOpCodes(editsStored)); +1 pending on the fix.
          Hide
          Sammi SammiChen added a comment -

          Thanks Kai Zheng & Rakesh R reviewing the patch!

          Following comments will be added later as the follow-ons. All other comments are addressed in the 003.patch

          3. You changes in DFSClient looks like some bug fix to existing codes.
          4. Refactor: getEcPolicy => getErasureCodingPolicy; AddECPolicyResponse => AddErasureCodingPolicyResponse
          5. Don't quite like the way to sort the map by creating a tree map. And also, could we improve ECSchema to ensure extraOptions is sorted already, so we don't need to consider doing it in every places? If you'd do this, please in separate issue.

          Show
          Sammi SammiChen added a comment - Thanks Kai Zheng & Rakesh R reviewing the patch! Following comments will be added later as the follow-ons. All other comments are addressed in the 003.patch 3. You changes in DFSClient looks like some bug fix to existing codes. 4. Refactor: getEcPolicy => getErasureCodingPolicy; AddECPolicyResponse => AddErasureCodingPolicyResponse 5. Don't quite like the way to sort the map by creating a tree map. And also, could we improve ECSchema to ensure extraOptions is sorted already, so we don't need to consider doing it in every places? If you'd do this, please in separate issue.
          Hide
          Sammi SammiChen added a comment -

          new editsStored raw file

          Show
          Sammi SammiChen added a comment - new editsStored raw file
          Hide
          rakeshr Rakesh R added a comment -

          Thanks SammiChen for the continuous efforts.

          I'm not very sure about the "Update javadocs" comments, is that because new parameter "logRetryCache" is added for involved API?

          Yes, good to could update javadoc,
          (1) with new param.

             * @param logRetryCache whether to record RPC ids in editlog for retry cache
             *                      rebuilding
          

          (2) Secondly, since you are touching FSDirErasureCodingOp functions, could you please add javadoc for the APIs which doesn't have details presently.

          FSDirErasureCodingOp#addErasureCodingPolicy, FSDirErasureCodingOp#removeErasureCodingPolicy, FSDirErasureCodingOp#enableErasureCodingPolicy, FSDirErasureCodingOp#disableErasureCodingPolicy functions.
          
          Show
          rakeshr Rakesh R added a comment - Thanks SammiChen for the continuous efforts. I'm not very sure about the "Update javadocs" comments, is that because new parameter "logRetryCache" is added for involved API? Yes, good to could update javadoc, (1) with new param. * @param logRetryCache whether to record RPC ids in editlog for retry cache * rebuilding (2) Secondly, since you are touching FSDirErasureCodingOp functions, could you please add javadoc for the APIs which doesn't have details presently. FSDirErasureCodingOp#addErasureCodingPolicy, FSDirErasureCodingOp#removeErasureCodingPolicy, FSDirErasureCodingOp#enableErasureCodingPolicy, FSDirErasureCodingOp#disableErasureCodingPolicy functions.
          Hide
          drankye Kai Zheng added a comment -

          Thanks SammiChen for working on this. Having looked into the work and it looks overall close.

          1. Overall, similar to HDFS-7859, this exposes lots of not-so-relevant changes already made in the patch or we need to make, so please feel free to open new issues to hold such changes separately. The essential changes for this issue is to log add/remove/enable/disable erasure coding policy.

          2. You local change in pom.xml and editsStored.xml.

          3. You changes in DFSClient looks like some bug fix to existing codes.

          4. Refactor: getEcPolicy => getErasureCodingPolicy; AddECPolicyResponse => AddErasureCodingPolicyResponse

          5. Don't quite like the way to sort the map by creating a tree map. And also, could we improve ECSchema to ensure extraOptions is sorted already, so we don't need to consider doing it in every places? If you'd do this, please in separate issue.

          +      // Sort extra options based on key
          +      extraOptions = new TreeMap<String, String>(extraOptions);
          

          6. Please use some non-meaningful options for the test purpose to avoid possible confusion, like "testOption1" or the like.

          +    Map<String, String> extraOptions = new HashMap<String, String>();
          +    extraOptions.put("padding", "0");
          +    extraOptions.put("recycle", "true");
          

          7. Please try to use the same order when dump fields of erasure coding policy.

          +  public static void writeErasureCodingPolicy(DataOutputStream out,
          +      ErasureCodingPolicy ecPolicy) throws IOException {
          +    writeInt(ecPolicy.getCellSize(), out);
          +    writeString(ecPolicy.getSchema().getCodecName(), out);
          +    writeInt(ecPolicy.getNumDataUnits(), out);
          +    writeInt(ecPolicy.getNumParityUnits(), out);
          }
          
          +      XMLUtils.addSaxString(contentHandler, "CODEC", ecPolicy.getCodecName());
          +      XMLUtils.addSaxString(contentHandler, "CELLSIZE",
          +          Integer.toString(ecPolicy.getCellSize()));
          +      XMLUtils.addSaxString(contentHandler, "DATAUNITS",
          +          Integer.toString(ecPolicy.getNumDataUnits()));
          +      XMLUtils.addSaxString(contentHandler, "PARITYUNITS",
          +          Integer.toString(ecPolicy.getNumParityUnits()));
          

          8. Not sure why we need to catch and convert the exception here, but not in other places. Better to pass the e instead of its

          {e.getMessage()}

          when convert to new IOException.

          +    case OP_ADD_ERASURE_CODING_POLICY:
          +      AddErasureCodingPolicyOp addOp = (AddErasureCodingPolicyOp) op;
          +      try {
          +        fsNamesys.getErasureCodingPolicyManager().addPolicy(
          +            addOp.getEcPolicy());
          +      } catch (HadoopIllegalArgumentException e) {
          +        throw new IOException("Add erasure coding policy failed for:" +
          +            e.getMessage());
          +      }
          
          Show
          drankye Kai Zheng added a comment - Thanks SammiChen for working on this. Having looked into the work and it looks overall close. 1. Overall, similar to HDFS-7859 , this exposes lots of not-so-relevant changes already made in the patch or we need to make, so please feel free to open new issues to hold such changes separately. The essential changes for this issue is to log add/remove/enable/disable erasure coding policy . 2. You local change in pom.xml and editsStored.xml . 3. You changes in DFSClient looks like some bug fix to existing codes. 4. Refactor: getEcPolicy => getErasureCodingPolicy ; AddECPolicyResponse => AddErasureCodingPolicyResponse 5. Don't quite like the way to sort the map by creating a tree map. And also, could we improve ECSchema to ensure extraOptions is sorted already, so we don't need to consider doing it in every places? If you'd do this, please in separate issue. + // Sort extra options based on key + extraOptions = new TreeMap< String , String >(extraOptions); 6. Please use some non-meaningful options for the test purpose to avoid possible confusion, like "testOption1" or the like. + Map< String , String > extraOptions = new HashMap< String , String >(); + extraOptions.put( "padding" , "0" ); + extraOptions.put( "recycle" , " true " ); 7. Please try to use the same order when dump fields of erasure coding policy. + public static void writeErasureCodingPolicy(DataOutputStream out, + ErasureCodingPolicy ecPolicy) throws IOException { + writeInt(ecPolicy.getCellSize(), out); + writeString(ecPolicy.getSchema().getCodecName(), out); + writeInt(ecPolicy.getNumDataUnits(), out); + writeInt(ecPolicy.getNumParityUnits(), out); } + XMLUtils.addSaxString(contentHandler, "CODEC" , ecPolicy.getCodecName()); + XMLUtils.addSaxString(contentHandler, "CELLSIZE" , + Integer .toString(ecPolicy.getCellSize())); + XMLUtils.addSaxString(contentHandler, "DATAUNITS" , + Integer .toString(ecPolicy.getNumDataUnits())); + XMLUtils.addSaxString(contentHandler, "PARITYUNITS" , + Integer .toString(ecPolicy.getNumParityUnits())); 8. Not sure why we need to catch and convert the exception here, but not in other places. Better to pass the e instead of its {e.getMessage()} when convert to new IOException. + case OP_ADD_ERASURE_CODING_POLICY: + AddErasureCodingPolicyOp addOp = (AddErasureCodingPolicyOp) op; + try { + fsNamesys.getErasureCodingPolicyManager().addPolicy( + addOp.getEcPolicy()); + } catch (HadoopIllegalArgumentException e) { + throw new IOException( "Add erasure coding policy failed for :" + + e.getMessage()); + }
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.
                trunk Compile Tests
          0 mvndep 1m 34s Maven dependency ordering for branch
          +1 mvninstall 13m 56s trunk passed
          +1 compile 14m 56s trunk passed
          +1 checkstyle 2m 15s trunk passed
          +1 mvnsite 2m 9s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-client-modules
          -1 findbugs 1m 46s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings.
          +1 javadoc 1m 29s trunk passed
                Patch Compile Tests
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 8m 1s the patch passed
          +1 compile 13m 3s the patch passed
          +1 javac 13m 3s the patch passed
          -0 checkstyle 2m 13s root: The patch generated 4 new + 1055 unchanged - 0 fixed = 1059 total (was 1055)
          +1 mvnsite 2m 17s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 3s The patch has no ill-formed XML file.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-client-modules
          +1 findbugs 4m 8s the patch passed
          +1 javadoc 1m 35s the patch passed
                Other Tests
          +1 unit 1m 19s hadoop-hdfs-client in the patch passed.
          -1 unit 123m 48s hadoop-hdfs in the patch failed.
          +1 unit 0m 37s hadoop-client-modules in the patch passed.
          +1 asflicense 0m 42s The patch does not generate ASF License warnings.
          199m 49s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure130
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting
            hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
            hadoop.hdfs.server.namenode.TestReconstructStripedBlocks
            hadoop.hdfs.web.TestWebHdfsTokens
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureToleration
            hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA
            hadoop.hdfs.TestLeaseRecoveryStriped
            hadoop.hdfs.TestClientProtocolForPipelineRecovery
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure210
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure170
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure200
            hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
            hadoop.hdfs.server.blockmanagement.TestBlockManager
            hadoop.hdfs.TestFileAppendRestart
            hadoop.hdfs.TestReadStripedFileWithMissingBlocks
            hadoop.hdfs.server.blockmanagement.TestBlockStatsMXBean
            hadoop.hdfs.TestEncryptedTransfer
            hadoop.hdfs.protocol.datatransfer.sasl.TestSaslDataTransfer
            hadoop.hdfs.server.namenode.TestNamenodeRetryCache
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure010
            hadoop.hdfs.TestFileCreationDelete
          Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue HDFS-12395
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12885999/HDFS-12395.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
          uname Linux 6751fc0d3f16 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b3a4d7d
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/21046/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/21046/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/21046/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/21046/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs hadoop-client-modules U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/21046/console
          Powered by Apache Yetus 0.6.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.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.       trunk Compile Tests 0 mvndep 1m 34s Maven dependency ordering for branch +1 mvninstall 13m 56s trunk passed +1 compile 14m 56s trunk passed +1 checkstyle 2m 15s trunk passed +1 mvnsite 2m 9s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-client-modules -1 findbugs 1m 46s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings. +1 javadoc 1m 29s trunk passed       Patch Compile Tests 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 8m 1s the patch passed +1 compile 13m 3s the patch passed +1 javac 13m 3s the patch passed -0 checkstyle 2m 13s root: The patch generated 4 new + 1055 unchanged - 0 fixed = 1059 total (was 1055) +1 mvnsite 2m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 3s The patch has no ill-formed XML file. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-client-modules +1 findbugs 4m 8s the patch passed +1 javadoc 1m 35s the patch passed       Other Tests +1 unit 1m 19s hadoop-hdfs-client in the patch passed. -1 unit 123m 48s hadoop-hdfs in the patch failed. +1 unit 0m 37s hadoop-client-modules in the patch passed. +1 asflicense 0m 42s The patch does not generate ASF License warnings. 199m 49s Reason Tests Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure130   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting   hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer   hadoop.hdfs.server.namenode.TestReconstructStripedBlocks   hadoop.hdfs.web.TestWebHdfsTokens   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureToleration   hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA   hadoop.hdfs.TestLeaseRecoveryStriped   hadoop.hdfs.TestClientProtocolForPipelineRecovery   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure210   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure170   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure200   hadoop.hdfs.server.namenode.ha.TestPipelinesFailover   hadoop.hdfs.server.blockmanagement.TestBlockManager   hadoop.hdfs.TestFileAppendRestart   hadoop.hdfs.TestReadStripedFileWithMissingBlocks   hadoop.hdfs.server.blockmanagement.TestBlockStatsMXBean   hadoop.hdfs.TestEncryptedTransfer   hadoop.hdfs.protocol.datatransfer.sasl.TestSaslDataTransfer   hadoop.hdfs.server.namenode.TestNamenodeRetryCache   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure010   hadoop.hdfs.TestFileCreationDelete Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue HDFS-12395 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12885999/HDFS-12395.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux 6751fc0d3f16 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b3a4d7d Default Java 1.8.0_144 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/21046/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/21046/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/21046/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/21046/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs hadoop-client-modules U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/21046/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Sammi SammiChen added a comment -

          002.patch uploaded. Here is the change list,
          1. fixed style issues
          2. fixed and improved the failed unit tests
          3. Rakesh's comments are addressed.

          Hi Rakesh R, I'm not very sure about the "Update javadocs" comments, is that because new parameter "logRetryCache" is added for involved API?

          I separately uploaded the patch and editsStored file. If you want to apply the patch locally, you can first apply the patch, then replace the file under "hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored" with the one in this JIRA.

          Show
          Sammi SammiChen added a comment - 002.patch uploaded. Here is the change list, 1. fixed style issues 2. fixed and improved the failed unit tests 3. Rakesh's comments are addressed. Hi Rakesh R , I'm not very sure about the "Update javadocs" comments, is that because new parameter "logRetryCache" is added for involved API? I separately uploaded the patch and editsStored file. If you want to apply the patch locally, you can first apply the patch, then replace the file under "hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored" with the one in this JIRA.
          Hide
          rakeshr Rakesh R added a comment -

          My understanding is CacheEntryWithPayload is used for API which has return object. CacheEntry is used fro API without return value.

          OK, fine.

          Show
          rakeshr Rakesh R added a comment - My understanding is CacheEntryWithPayload is used for API which has return object. CacheEntry is used fro API without return value. OK, fine.
          Hide
          Sammi SammiChen added a comment -

          Thanks Rakesh R for the quick review!

          I could see CacheEntry and CacheEntryWithPayload used in the patch. Is it intentionally used differently for different cases?

          My understanding is CacheEntryWithPayload is used for API which has return object. CacheEntry is used fro API without return value.

          Can we move checkOperation() and checkNameNodeSafeMode() checks outside for loop. If it throws exception, then #addErasureCodingPolicies() operation won't be atomic anyway, right? Atomic op, I meant either API should consider all policies and shouldn't add policies partially.

          It's a good suggestion to move checkOperation() and checkNameNodeSafeMode() checks outside for loop. For addErasureCodingPolicies API itself, atomic is not a goal.

          All other comments will be addressed in next patch.

          Show
          Sammi SammiChen added a comment - Thanks Rakesh R for the quick review! I could see CacheEntry and CacheEntryWithPayload used in the patch. Is it intentionally used differently for different cases? My understanding is CacheEntryWithPayload is used for API which has return object. CacheEntry is used fro API without return value. Can we move checkOperation() and checkNameNodeSafeMode() checks outside for loop. If it throws exception, then #addErasureCodingPolicies() operation won't be atomic anyway, right? Atomic op, I meant either API should consider all policies and shouldn't add policies partially. It's a good suggestion to move checkOperation() and checkNameNodeSafeMode() checks outside for loop. For addErasureCodingPolicies API itself, atomic is not a goal. All other comments will be addressed in next patch.
          Hide
          rakeshr Rakesh R added a comment -

          Thanks SammiChen for the patch. Adding few comments on the patch:

          1. Missing new EC opcodes in editsStored.xml.
          2. Update javadocs for:
            FSNamesystem#addErasureCodingPolicies, FSNamesystem#removeErasureCodingPolicy, FSNamesystem#enableErasureCodingPolicy, FSNamesystem#disableErasureCodingPolicy
            
            FSDirErasureCodingOp#addErasureCodingPolicy, FSDirErasureCodingOp#removeErasureCodingPolicy, FSDirErasureCodingOp#enableErasureCodingPolicy, FSDirErasureCodingOp#disableErasureCodingPolicy functions.
            
          3. I could see CacheEntry and CacheEntryWithPayload used in the patch. Is it intentionally used differently for different cases?
          4. Can we move checkOperation() and checkNameNodeSafeMode() checks outside for loop. If it throws exception, then #addErasureCodingPolicies() operation won't be atomic anyway, right? Atomic op, I meant either API should consider all policies and shouldn't add policies partially.
            FSNamesystem#addErasureCodingPolicies()
                      ......
                  for (ErasureCodingPolicy policy : policies) {
                    try {
                      checkOperation(OperationCategory.WRITE);
                      checkNameNodeSafeMode("Cannot add erasure coding policy");
                      ErasureCodingPolicy newPolicy =
                          FSDirErasureCodingOp.addErasureCodingPolicy(this, policy,
                              logRetryCache);
            
          5. Looks like test case failures are related, please take care.
          Show
          rakeshr Rakesh R added a comment - Thanks SammiChen for the patch. Adding few comments on the patch: Missing new EC opcodes in editsStored.xml . Update javadocs for: FSNamesystem#addErasureCodingPolicies, FSNamesystem#removeErasureCodingPolicy, FSNamesystem#enableErasureCodingPolicy, FSNamesystem#disableErasureCodingPolicy FSDirErasureCodingOp#addErasureCodingPolicy, FSDirErasureCodingOp#removeErasureCodingPolicy, FSDirErasureCodingOp#enableErasureCodingPolicy, FSDirErasureCodingOp#disableErasureCodingPolicy functions. I could see CacheEntry and CacheEntryWithPayload used in the patch. Is it intentionally used differently for different cases? Can we move checkOperation() and checkNameNodeSafeMode() checks outside for loop. If it throws exception, then #addErasureCodingPolicies() operation won't be atomic anyway, right? Atomic op, I meant either API should consider all policies and shouldn't add policies partially. FSNamesystem#addErasureCodingPolicies() ...... for (ErasureCodingPolicy policy : policies) { try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode( "Cannot add erasure coding policy" ); ErasureCodingPolicy newPolicy = FSDirErasureCodingOp.addErasureCodingPolicy( this , policy, logRetryCache); Looks like test case failures are related, please take care.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
                Prechecks
          +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.
                trunk Compile Tests
          0 mvndep 0m 29s Maven dependency ordering for branch
          +1 mvninstall 14m 4s trunk passed
          +1 compile 1m 28s trunk passed
          +1 checkstyle 0m 57s trunk passed
          +1 mvnsite 1m 30s trunk passed
          +1 findbugs 3m 25s trunk passed
          +1 javadoc 1m 30s trunk passed
                Patch Compile Tests
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 2m 22s the patch passed
          +1 compile 2m 22s the patch passed
          +1 javac 2m 22s the patch passed
          -0 checkstyle 1m 14s hadoop-hdfs-project: The patch generated 13 new + 964 unchanged - 0 fixed = 977 total (was 964)
          +1 mvnsite 2m 16s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 4m 53s the patch passed
          +1 javadoc 1m 13s the patch passed
                Other Tests
          +1 unit 1m 38s hadoop-hdfs-client in the patch passed.
          -1 unit 90m 6s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          132m 40s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestDFSClientRetries
            hadoop.hdfs.TestClientProtocolForPipelineRecovery
            hadoop.hdfs.TestEncryptedTransfer
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure000
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure070
            hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure020
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure090
            hadoop.hdfs.TestLeaseRecoveryStriped
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure040
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting
            hadoop.hdfs.TestReadStripedFileWithDecoding
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure150
          Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue HDFS-12395
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12885349/HDFS-12395.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9c5d7d3a6485 3.13.0-123-generic #172-Ubuntu SMP Mon Jun 26 18:04:35 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 5dba545
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20999/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/20999/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20999/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20999/console
          Powered by Apache Yetus 0.6.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 14s Docker mode activated.       Prechecks +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.       trunk Compile Tests 0 mvndep 0m 29s Maven dependency ordering for branch +1 mvninstall 14m 4s trunk passed +1 compile 1m 28s trunk passed +1 checkstyle 0m 57s trunk passed +1 mvnsite 1m 30s trunk passed +1 findbugs 3m 25s trunk passed +1 javadoc 1m 30s trunk passed       Patch Compile Tests 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 2m 22s the patch passed +1 compile 2m 22s the patch passed +1 javac 2m 22s the patch passed -0 checkstyle 1m 14s hadoop-hdfs-project: The patch generated 13 new + 964 unchanged - 0 fixed = 977 total (was 964) +1 mvnsite 2m 16s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 4m 53s the patch passed +1 javadoc 1m 13s the patch passed       Other Tests +1 unit 1m 38s hadoop-hdfs-client in the patch passed. -1 unit 90m 6s hadoop-hdfs in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 132m 40s Reason Tests Failed junit tests hadoop.hdfs.TestDFSClientRetries   hadoop.hdfs.TestClientProtocolForPipelineRecovery   hadoop.hdfs.TestEncryptedTransfer   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure000   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure070   hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure020   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure090   hadoop.hdfs.TestLeaseRecoveryStriped   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure040   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting   hadoop.hdfs.TestReadStripedFileWithDecoding   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure150 Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue HDFS-12395 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12885349/HDFS-12395.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9c5d7d3a6485 3.13.0-123-generic #172-Ubuntu SMP Mon Jun 26 18:04:35 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5dba545 Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20999/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/20999/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20999/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20999/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Sammi SammiChen added a comment -

          Initial patch

          Show
          Sammi SammiChen added a comment - Initial patch

            People

            • Assignee:
              Sammi SammiChen
              Reporter:
              Sammi SammiChen
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development