Details

    • Target Version/s:
    • Hadoop Flags:
      Incompatible change
    • Release Note:
      HDFS will now restrict the set of erasure coding policies that can be set by users. The set of allowed policies can be configured via "dfs.namenode.ec.policies.enabled" on the NameNode. Please see the documentation for more details.

      Description

      Filing based on discussion in HDFS-8095. A user might specify a policy that is not appropriate for the cluster, e.g. a RS (10,4) policy when the cluster only has 10 nodes. The NN should only allow the client to choose from a pre-approved list determined by the cluster administrator.

      1. HDFS-11314.001.patch
        22 kB
        Andrew Wang
      2. HDFS-11314.002.patch
        39 kB
        Andrew Wang
      3. HDFS-11314.003.patch
        55 kB
        Andrew Wang
      4. HDFS-11314.004.patch
        56 kB
        Andrew Wang
      5. HDFS-11314.005.patch
        56 kB
        Andrew Wang
      6. HDFS-11314.006.patch
        56 kB
        Andrew Wang

        Issue Links

          Activity

          Hide
          Sammi SammiChen added a comment -

          So if a user sets an EC policy on a directory/file, NameNode should check whether this EC policy can be fulfilled by current data nodes. If NameNode can't fulfill the EC policy, then fail it. Correct?
          Currently, if a cluster has 10 nodes, a RS(10,4) policy will succeed. It will fail unless the cluster only has 9 nodes. Should we raise the bar so that a EC policy will succeed only when the cluster has more datanodes that policy's (data + parity) block number?
          "a pre-approved list determined by the cluster administrator" can I know where or how the cluster administrator provide such list?

          Show
          Sammi SammiChen added a comment - So if a user sets an EC policy on a directory/file, NameNode should check whether this EC policy can be fulfilled by current data nodes. If NameNode can't fulfill the EC policy, then fail it. Correct? Currently, if a cluster has 10 nodes, a RS(10,4) policy will succeed. It will fail unless the cluster only has 9 nodes. Should we raise the bar so that a EC policy will succeed only when the cluster has more datanodes that policy's (data + parity) block number? "a pre-approved list determined by the cluster administrator" can I know where or how the cluster administrator provide such list?
          Hide
          andrew.wang Andrew Wang added a comment -

          Sorry I missed this comment, JIRA has been really unhappy over the last week.

          IMO this JIRA requires HDFS-7859 which would persist EC policies into the FSImage. This would is a nice way for the admin to provide a list of acceptable policies.

          Enforcing based on # nodes or # racks is tougher, since we don't know if the admin wants node or rack fault tolerance, and the size of the cluster can change.

          Show
          andrew.wang Andrew Wang added a comment - Sorry I missed this comment, JIRA has been really unhappy over the last week. IMO this JIRA requires HDFS-7859 which would persist EC policies into the FSImage. This would is a nice way for the admin to provide a list of acceptable policies. Enforcing based on # nodes or # racks is tougher, since we don't know if the admin wants node or rack fault tolerance, and the size of the cluster can change.
          Hide
          andrew.wang Andrew Wang added a comment -

          Marking ec must do's as blockers for alpha3

          Show
          andrew.wang Andrew Wang added a comment - Marking ec must do's as blockers for alpha3
          Hide
          andrew.wang Andrew Wang added a comment -

          Hi Chen Liang do you mind if I take this one? I've been discussing this with some others on HDFS-7859, and think I have a good handle on what to do here.

          Show
          andrew.wang Andrew Wang added a comment - Hi Chen Liang do you mind if I take this one? I've been discussing this with some others on HDFS-7859 , and think I have a good handle on what to do here.
          Hide
          vagarychen Chen Liang added a comment -

          Hi Andrew Wang, sorry I've been a bit busy on some other works recently and haven't got a chance to work on this. Sure, please take this as you like.

          Show
          vagarychen Chen Liang added a comment - Hi Andrew Wang , sorry I've been a bit busy on some other works recently and haven't got a chance to work on this. Sure, please take this as you like.
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks, assigning to myself.

          Show
          andrew.wang Andrew Wang added a comment - Thanks, assigning to myself.
          Hide
          andrew.wang Andrew Wang added a comment -

          Patch attached, this adds a new comma-delimited config key that allows users to specify a list of enabled policies that are enforced by the NN.

          Show
          andrew.wang Andrew Wang added a comment - Patch attached, this adds a new comma-delimited config key that allows users to specify a list of enabled policies that are enforced by the NN.
          Hide
          andrew.wang Andrew Wang added a comment -

          I still want to add a few more unit tests, but posting this to get a precommit run.

          Show
          andrew.wang Andrew Wang added a comment - I still want to add a few more unit tests, but posting this to get a precommit run.
          Hide
          andrew.wang Andrew Wang added a comment -

          Glad I added the additional tests, I caught some places where we were querying the set of active policies rather than the set of system policies. I cleaned up the getters in ECPolicyManager to clarify this.

          Show
          andrew.wang Andrew Wang added a comment - Glad I added the additional tests, I caught some places where we were querying the set of active policies rather than the set of system policies. I cleaned up the getters in ECPolicyManager to clarify this.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



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



          Subsystem Report/Notes
          JIRA Issue HDFS-11314
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12855492/HDFS-11314.002.patch
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18501/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 0s Docker mode activated. -1 patch 0m 8s HDFS-11314 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HDFS-11314 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12855492/HDFS-11314.002.patch Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18501/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          Cancelling patch while we wait for HDFS-11416, they conflict and 11416 should be easier to get in first.

          Show
          andrew.wang Andrew Wang added a comment - Cancelling patch while we wait for HDFS-11416 , they conflict and 11416 should be easier to get in first.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 10 new or modified test files.
          +1 mvninstall 13m 35s trunk passed
          +1 compile 0m 58s trunk passed
          +1 checkstyle 0m 44s trunk passed
          +1 mvnsite 0m 58s trunk passed
          +1 mvneclipse 0m 18s trunk passed
          +1 findbugs 1m 54s trunk passed
          +1 javadoc 0m 42s trunk passed
          +1 mvninstall 0m 49s the patch passed
          +1 compile 0m 55s the patch passed
          +1 javac 0m 55s the patch passed
          -0 checkstyle 0m 43s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 709 unchanged - 1 fixed = 713 total (was 710)
          +1 mvnsite 1m 1s the patch passed
          +1 mvneclipse 0m 12s 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.
          +1 findbugs 2m 8s the patch passed
          +1 javadoc 0m 40s the patch passed
          -1 unit 70m 55s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          98m 30s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestMaintenanceState
            hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer
            hadoop.hdfs.server.datanode.TestDirectoryScanner
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HDFS-11314
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12855469/HDFS-11314.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux df02c401942d 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 899d5c4
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18497/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/18497/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18497/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18497/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 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 10 new or modified test files. +1 mvninstall 13m 35s trunk passed +1 compile 0m 58s trunk passed +1 checkstyle 0m 44s trunk passed +1 mvnsite 0m 58s trunk passed +1 mvneclipse 0m 18s trunk passed +1 findbugs 1m 54s trunk passed +1 javadoc 0m 42s trunk passed +1 mvninstall 0m 49s the patch passed +1 compile 0m 55s the patch passed +1 javac 0m 55s the patch passed -0 checkstyle 0m 43s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 709 unchanged - 1 fixed = 713 total (was 710) +1 mvnsite 1m 1s the patch passed +1 mvneclipse 0m 12s 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. +1 findbugs 2m 8s the patch passed +1 javadoc 0m 40s the patch passed -1 unit 70m 55s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 98m 30s Reason Tests Failed junit tests hadoop.hdfs.TestMaintenanceState   hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer   hadoop.hdfs.server.datanode.TestDirectoryScanner   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11314 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12855469/HDFS-11314.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux df02c401942d 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 899d5c4 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18497/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/18497/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18497/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18497/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          Patch attached. Here's a summary of changes:

          • Differentiate "system" from "enabled" policies. This meant renaming "active" to "enabled" in some APIs, comments, variable names.
          • Update documentation to talk about EC policies as a concept, and how to enable them.
          • Exception and other messages telling the user how to enable EC policies.
          • Update tests to enable additional policies where required.

          One question for reviewers is whether we should have a default policy enabled or not. The set of enabled policies has to be configured correctly for the cluster at hand, so it might be better to leave it empty and force users to choose the right value. This would require some more test updates, so I might ask to do that as a follow-on change.

          Show
          andrew.wang Andrew Wang added a comment - Patch attached. Here's a summary of changes: Differentiate "system" from "enabled" policies. This meant renaming "active" to "enabled" in some APIs, comments, variable names. Update documentation to talk about EC policies as a concept, and how to enable them. Exception and other messages telling the user how to enable EC policies. Update tests to enable additional policies where required. One question for reviewers is whether we should have a default policy enabled or not. The set of enabled policies has to be configured correctly for the cluster at hand, so it might be better to leave it empty and force users to choose the right value. This would require some more test updates, so I might ask to do that as a follow-on change.
          Hide
          rakeshr Rakesh R added a comment -

          Thanks Andrew Wang, enabling concept looks good to me.

          One question for reviewers is whether we should have a default policy enabled or not. The set of enabled policies has to be configured correctly for the cluster at hand, so it might be better to leave it empty and force users to choose the right value. This would require some more test updates, so I might ask to do that as a follow-on change.

          I'd also prefer keep the list empty and lets users configure the right values based on their topology. Agreed to push this changes via separate jira task.

          Adding few points to discuss,

          1. I'm referring the below quoted line from HDFSErasureCoding.md doc changes section in the patch.
            It does not affect the behavior of already set file or directory-level EC policies.

            IIUC, talking about a case where user disabled an ec policy which was enabled earlier and has set of ec files created with that ec policy. Could you please point me the way Namenode handles this during restarts and read from edit log. It would be good to add test case if not added.

          2. Whats your opinion to provide a facility to dynamically/reconfigure enabling the policies with out needing to restart Namenode?
          Show
          rakeshr Rakesh R added a comment - Thanks Andrew Wang , enabling concept looks good to me. One question for reviewers is whether we should have a default policy enabled or not. The set of enabled policies has to be configured correctly for the cluster at hand, so it might be better to leave it empty and force users to choose the right value. This would require some more test updates, so I might ask to do that as a follow-on change. I'd also prefer keep the list empty and lets users configure the right values based on their topology. Agreed to push this changes via separate jira task. Adding few points to discuss, I'm referring the below quoted line from HDFSErasureCoding.md doc changes section in the patch. It does not affect the behavior of already set file or directory-level EC policies. IIUC, talking about a case where user disabled an ec policy which was enabled earlier and has set of ec files created with that ec policy. Could you please point me the way Namenode handles this during restarts and read from edit log. It would be good to add test case if not added. Whats your opinion to provide a facility to dynamically/reconfigure enabling the policies with out needing to restart Namenode?
          Hide
          andrew.wang Andrew Wang added a comment -

          Hi Rakesh, thanks for reviewing!

          I'd also prefer keep the list empty and lets users configure the right values based on their topology.

          Great, I filed HDFS-11505. I'll handle this after we get this one in.

          Could you please point me the way Namenode handles this during restarts and read from edit log. It would be good to add test case if not added.

          I extended TestErasureCodingPolicies#testBasicSetECPolicy, little snippets here:

          +    // Verify that policies are successfully loaded even when policies
          +    // are disabled
          +    cluster.getConfiguration(0).set(
          +        DFSConfigKeys.DFS_NAMENODE_EC_POLICIES_ENABLED_KEY, "");
          +    cluster.restartNameNodes();
          +    cluster.waitActive();
          
          +    // Also check loading disabled EC policies from fsimage
          +    fs.setSafeMode(HdfsConstants.SafeModeAction.SAFEMODE_ENTER);
          +    fs.saveNamespace();
          +    fs.setSafeMode(HdfsConstants.SafeModeAction.SAFEMODE_LEAVE);
          +    cluster.restartNameNodes();
          

          Whats your opinion to provide a facility to dynamically/reconfigure enabling the policies with out needing to restart Namenode?

          I think changing the set of EC policies is a pretty rare operation, so it's not that important.

          One related nice work item would be doing this for the replication throttles, since that's an issue even now.

          Show
          andrew.wang Andrew Wang added a comment - Hi Rakesh, thanks for reviewing! I'd also prefer keep the list empty and lets users configure the right values based on their topology. Great, I filed HDFS-11505 . I'll handle this after we get this one in. Could you please point me the way Namenode handles this during restarts and read from edit log. It would be good to add test case if not added. I extended TestErasureCodingPolicies#testBasicSetECPolicy, little snippets here: + // Verify that policies are successfully loaded even when policies + // are disabled + cluster.getConfiguration(0).set( + DFSConfigKeys.DFS_NAMENODE_EC_POLICIES_ENABLED_KEY, ""); + cluster.restartNameNodes(); + cluster.waitActive(); + // Also check loading disabled EC policies from fsimage + fs.setSafeMode(HdfsConstants.SafeModeAction.SAFEMODE_ENTER); + fs.saveNamespace(); + fs.setSafeMode(HdfsConstants.SafeModeAction.SAFEMODE_LEAVE); + cluster.restartNameNodes(); Whats your opinion to provide a facility to dynamically/reconfigure enabling the policies with out needing to restart Namenode? I think changing the set of EC policies is a pretty rare operation, so it's not that important. One related nice work item would be doing this for the replication throttles, since that's an issue even now.
          Hide
          andrew.wang Andrew Wang added a comment -

          Here's a patch to fix the TestOIV failure, had to enable an EC policy.

          Show
          andrew.wang Andrew Wang added a comment - Here's a patch to fix the TestOIV failure, had to enable an EC policy.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 17 new or modified test files.
          +1 mvninstall 11m 55s trunk passed
          +1 compile 0m 51s trunk passed
          +1 checkstyle 0m 38s trunk passed
          +1 mvnsite 0m 48s trunk passed
          +1 mvneclipse 0m 11s trunk passed
          +1 findbugs 1m 38s trunk passed
          +1 javadoc 0m 37s trunk passed
          +1 mvninstall 0m 44s the patch passed
          +1 compile 0m 39s the patch passed
          +1 javac 0m 39s the patch passed
          -0 checkstyle 0m 36s hadoop-hdfs-project/hadoop-hdfs: The patch generated 8 new + 771 unchanged - 6 fixed = 779 total (was 777)
          +1 mvnsite 0m 47s the patch passed
          +1 mvneclipse 0m 9s 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.
          +1 findbugs 1m 44s the patch passed
          +1 javadoc 0m 35s the patch passed
          -1 unit 70m 1s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 14s The patch does not generate ASF License warnings.
          93m 34s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting
            hadoop.hdfs.web.TestWebHdfsTimeouts
          Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HDFS-11314
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12856372/HDFS-11314.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 0568dc0418e4 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 5e74196
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18599/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/18599/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18599/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18599/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 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 17 new or modified test files. +1 mvninstall 11m 55s trunk passed +1 compile 0m 51s trunk passed +1 checkstyle 0m 38s trunk passed +1 mvnsite 0m 48s trunk passed +1 mvneclipse 0m 11s trunk passed +1 findbugs 1m 38s trunk passed +1 javadoc 0m 37s trunk passed +1 mvninstall 0m 44s the patch passed +1 compile 0m 39s the patch passed +1 javac 0m 39s the patch passed -0 checkstyle 0m 36s hadoop-hdfs-project/hadoop-hdfs: The patch generated 8 new + 771 unchanged - 6 fixed = 779 total (was 777) +1 mvnsite 0m 47s the patch passed +1 mvneclipse 0m 9s 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. +1 findbugs 1m 44s the patch passed +1 javadoc 0m 35s the patch passed -1 unit 70m 1s hadoop-hdfs in the patch failed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 93m 34s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting   hadoop.hdfs.web.TestWebHdfsTimeouts Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2 Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11314 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12856372/HDFS-11314.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 0568dc0418e4 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5e74196 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18599/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/18599/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18599/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18599/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          drankye Kai Zheng added a comment -

          I agree it's much safer to have admin configure and ensure the valid policies. The list should only affect the choices that users would choose for other files/folders, and incur no impact for existing erasure coded files/folders. Thanks Andrew Wang for the great work!

          One question and minor, noticed pretty many changes like follows and wondered why. If we would allow user defined policies in future, due to whatever considerations like new codec, new option even for existing codec and etc., I guess the more general getPolicyByPolicyID name would be better.

          -    return ErasureCodingPolicyManager.getPolicyByPolicyID(
          +    return ErasureCodingPolicyManager.getSystemPolicyByID(
          
          Show
          drankye Kai Zheng added a comment - I agree it's much safer to have admin configure and ensure the valid policies. The list should only affect the choices that users would choose for other files/folders, and incur no impact for existing erasure coded files/folders. Thanks Andrew Wang for the great work! One question and minor, noticed pretty many changes like follows and wondered why. If we would allow user defined policies in future, due to whatever considerations like new codec, new option even for existing codec and etc., I guess the more general getPolicyByPolicyID name would be better. - return ErasureCodingPolicyManager.getPolicyByPolicyID( + return ErasureCodingPolicyManager.getSystemPolicyByID(
          Hide
          andrew.wang Andrew Wang added a comment -

          New patch to address checkstyle errors.

          Show
          andrew.wang Andrew Wang added a comment - New patch to address checkstyle errors.
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks for reviewing Kai! Regarding the naming, I was trying to differentiate between:

          1. The set of all possible policies
          2. The set of enabled policies

          I reused "system" to refer to the first set, while using "enabled" to refer to the second set. I forgot that "system" specifically refers to the hardcoded policies, so let's make the names more general as you say.

          I shortened your proposal a little bit if that's okay, named the getters getPolicyByName and getPolicyByID.

          Show
          andrew.wang Andrew Wang added a comment - Thanks for reviewing Kai! Regarding the naming, I was trying to differentiate between: The set of all possible policies The set of enabled policies I reused "system" to refer to the first set, while using "enabled" to refer to the second set. I forgot that "system" specifically refers to the hardcoded policies, so let's make the names more general as you say. I shortened your proposal a little bit if that's okay, named the getters getPolicyByName and getPolicyByID .
          Hide
          drankye Kai Zheng added a comment -

          Thanks for your clarifying and consideration, Andrew. Yes renaming the getters should work.

          Show
          drankye Kai Zheng added a comment - Thanks for your clarifying and consideration, Andrew. Yes renaming the getters should work.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 31s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 17 new or modified test files.
          +1 mvninstall 14m 38s trunk passed
          +1 compile 0m 55s trunk passed
          +1 checkstyle 0m 45s trunk passed
          +1 mvnsite 0m 59s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 1m 54s trunk passed
          +1 javadoc 0m 42s trunk passed
          +1 mvninstall 0m 58s the patch passed
          +1 compile 0m 52s the patch passed
          +1 javac 0m 52s the patch passed
          -0 checkstyle 0m 45s hadoop-hdfs-project/hadoop-hdfs: The patch generated 3 new + 772 unchanged - 6 fixed = 775 total (was 778)
          +1 mvnsite 0m 57s the patch passed
          +1 mvneclipse 0m 13s 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.
          +1 findbugs 2m 0s the patch passed
          +1 javadoc 0m 40s the patch passed
          -1 unit 127m 43s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          156m 35s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestReadStripedFileWithMissingBlocks
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.server.namenode.ha.TestDFSUpgradeWithHA
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting
            hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery
          Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HDFS-11314
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12856411/HDFS-11314.006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 19a2a27f5dda 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 28daaf0
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18631/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/18631/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18631/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18631/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 31s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 17 new or modified test files. +1 mvninstall 14m 38s trunk passed +1 compile 0m 55s trunk passed +1 checkstyle 0m 45s trunk passed +1 mvnsite 0m 59s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 54s trunk passed +1 javadoc 0m 42s trunk passed +1 mvninstall 0m 58s the patch passed +1 compile 0m 52s the patch passed +1 javac 0m 52s the patch passed -0 checkstyle 0m 45s hadoop-hdfs-project/hadoop-hdfs: The patch generated 3 new + 772 unchanged - 6 fixed = 775 total (was 778) +1 mvnsite 0m 57s the patch passed +1 mvneclipse 0m 13s 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. +1 findbugs 2m 0s the patch passed +1 javadoc 0m 40s the patch passed -1 unit 127m 43s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 156m 35s Reason Tests Failed junit tests hadoop.hdfs.TestReadStripedFileWithMissingBlocks   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.namenode.ha.TestDFSUpgradeWithHA   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting   hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2 Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11314 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12856411/HDFS-11314.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 19a2a27f5dda 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 28daaf0 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18631/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/18631/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18631/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18631/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          Test failures look unrelated and checkstyle do not apply. Could I get a final +1? Thanks.

          Show
          andrew.wang Andrew Wang added a comment - Test failures look unrelated and checkstyle do not apply. Could I get a final +1? Thanks.
          Hide
          drankye Kai Zheng added a comment -

          The latest patch LGTM and +1. Thanks Andrew for the nice work!

          Show
          drankye Kai Zheng added a comment - The latest patch LGTM and +1. Thanks Andrew for the nice work!
          Hide
          andrew.wang Andrew Wang added a comment -

          Committed to trunk, thanks for the reviews all! Had to do a little fixup since I also just committed the OIV patch from Manoj.

          Show
          andrew.wang Andrew Wang added a comment - Committed to trunk, thanks for the reviews all! Had to do a little fixup since I also just committed the OIV patch from Manoj.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11377 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11377/)
          HDFS-11314. Enforce set of enabled EC policies on the NameNode. (wang: rev 33a38a534110de454662256545a7f4c075d328c8)

          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestUnsetAndChangeDirectoryEcPolicy.java
          • (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/ErasureCodingPolicyManager.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSXORStripedOutputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSImage.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRSDefault10x4StripedOutputStreamWithFailure.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testErasureCodingConf.xml
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStreamWithFailure.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/ECAdmin.java
          • (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEnabledECPolicies.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRSDefault10x4StripedOutputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestErasureCodingCLI.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRSDefault10x4StripedInputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestErasureCodingPolicies.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSXORStripedOutputStreamWithFailure.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.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/TestDFSXORStripedInputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.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/TestDFSStripedOutputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSErasureCoding.md
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11377 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11377/ ) HDFS-11314 . Enforce set of enabled EC policies on the NameNode. (wang: rev 33a38a534110de454662256545a7f4c075d328c8) (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestUnsetAndChangeDirectoryEcPolicy.java (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/ErasureCodingPolicyManager.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSXORStripedOutputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSImage.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRSDefault10x4StripedOutputStreamWithFailure.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testErasureCodingConf.xml (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedOutputStreamWithFailure.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/ECAdmin.java (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEnabledECPolicies.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRSDefault10x4StripedOutputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestErasureCodingCLI.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSRSDefault10x4StripedInputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestErasureCodingPolicies.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSXORStripedOutputStreamWithFailure.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.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/TestDFSXORStripedInputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.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/TestDFSStripedOutputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSErasureCoding.md

            People

            • Assignee:
              andrew.wang Andrew Wang
              Reporter:
              andrew.wang Andrew Wang
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development