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

Cleanup unused DataNode#checkDiskErrorAsync()

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-alpha2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      After HDFS-11274, we will not trigger checking all datanode volumes upon IO failure on a single volume. This makes the original implementation DataNode#checkDiskErrorAsync and DatasetVolumeChecker#checkAllVolumesAsync() not used in any of the production code.

      This ticket is opened to remove these unused code and related tests if any.

      1. HDFS-11279.000.patch
        14 kB
        Hanisha Koneru
      2. HDFS-11279.001.patch
        15 kB
        Hanisha Koneru
      3. HDFS-11279.002.patch
        14 kB
        Hanisha Koneru

        Activity

        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 14s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        +1 mvninstall 12m 38s trunk passed
        +1 compile 0m 46s trunk passed
        +1 checkstyle 0m 29s trunk passed
        +1 mvnsite 0m 50s trunk passed
        +1 mvneclipse 0m 13s trunk passed
        +1 findbugs 1m 42s trunk passed
        +1 javadoc 0m 39s trunk passed
        +1 mvninstall 0m 45s the patch passed
        +1 compile 0m 44s the patch passed
        +1 javac 0m 44s the patch passed
        +1 checkstyle 0m 26s hadoop-hdfs-project/hadoop-hdfs: The patch generated 0 new + 230 unchanged - 1 fixed = 230 total (was 231)
        +1 mvnsite 0m 50s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 46s the patch passed
        +1 javadoc 0m 37s the patch passed
        -1 unit 78m 48s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 20s The patch does not generate ASF License warnings.
        103m 12s



        Reason Tests
        Failed junit tests hadoop.hdfs.security.TestDelegationTokenForProxyUser
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure190



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11279
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845112/HDFS-11279.000.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 2ccf147204e5 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 95c2c24
        Default Java 1.8.0_111
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/17989/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17989/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17989/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 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. +1 mvninstall 12m 38s trunk passed +1 compile 0m 46s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 0m 50s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 42s trunk passed +1 javadoc 0m 39s trunk passed +1 mvninstall 0m 45s the patch passed +1 compile 0m 44s the patch passed +1 javac 0m 44s the patch passed +1 checkstyle 0m 26s hadoop-hdfs-project/hadoop-hdfs: The patch generated 0 new + 230 unchanged - 1 fixed = 230 total (was 231) +1 mvnsite 0m 50s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 46s the patch passed +1 javadoc 0m 37s the patch passed -1 unit 78m 48s hadoop-hdfs in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 103m 12s Reason Tests Failed junit tests hadoop.hdfs.security.TestDelegationTokenForProxyUser   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure190 Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11279 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845112/HDFS-11279.000.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2ccf147204e5 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 95c2c24 Default Java 1.8.0_111 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/17989/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17989/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17989/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        xyao Xiaoyu Yao added a comment -

        Thanks Hanisha Koneru for working on this. The patch looks pretty good to me. I just have two NITs, +1 otherwise.

        Datanode.java
        NIT: Line50, import can be moved down to follow the alphabet order
        importorg.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsVolumeImpl;

        NIT: Line 2470, can you add @VisibleForTesting for DataNode#getVolume?
        Or I would suggest moving DataNode#getVolume(File basePath) into a test utility class such as DataNodeTestUtils#getVolume

        Public FsVolumeImpl getVolume(DataNode dn,File basePath)
        
        Show
        xyao Xiaoyu Yao added a comment - Thanks Hanisha Koneru for working on this. The patch looks pretty good to me. I just have two NITs, +1 otherwise. Datanode.java NIT: Line50, import can be moved down to follow the alphabet order importorg.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsVolumeImpl; NIT: Line 2470, can you add @VisibleForTesting for DataNode#getVolume? Or I would suggest moving DataNode#getVolume(File basePath) into a test utility class such as DataNodeTestUtils#getVolume Public FsVolumeImpl getVolume(DataNode dn,File basePath)
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Thank you Xiaoyu Yao for reviewing the patch. I have addressed your comments in patch v01.

        Show
        hanishakoneru Hanisha Koneru added a comment - Thank you Xiaoyu Yao for reviewing the patch. I have addressed your comments in patch v01.
        Hide
        xyao Xiaoyu Yao added a comment -

        Thanks Hanisha Koneru for the update. +1 pending Jenkins.
        There is a unused import in DataNode.java which we can fix at commit time to save a Jenkins run.

        Show
        xyao Xiaoyu Yao added a comment - Thanks Hanisha Koneru for the update. +1 pending Jenkins. There is a unused import in DataNode.java which we can fix at commit time to save a Jenkins run.
        Hide
        liuml07 Mingliang Liu added a comment -

        import java.io.File; in DataNode seems unnecessary? It will be good if we keep the javadoc for public test helper method getVolume(). Other than that it looks good to me.

        Show
        liuml07 Mingliang Liu added a comment - import java.io.File; in DataNode seems unnecessary? It will be good if we keep the javadoc for public test helper method getVolume() . Other than that it looks good to me.
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Thank you Xiaoyu Yao and Mingliang Liu. I have removed the unused import and added the javadoc back in v02.

        Show
        hanishakoneru Hanisha Koneru added a comment - Thank you Xiaoyu Yao and Mingliang Liu . I have removed the unused import and added the javadoc back in v02.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 15s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.
        +1 mvninstall 14m 48s trunk passed
        +1 compile 1m 1s trunk passed
        +1 checkstyle 0m 34s trunk passed
        +1 mvnsite 1m 3s trunk passed
        +1 mvneclipse 0m 15s trunk passed
        +1 findbugs 2m 1s trunk passed
        +1 javadoc 0m 47s trunk passed
        +1 mvninstall 0m 55s the patch passed
        +1 compile 0m 51s the patch passed
        +1 javac 0m 51s the patch passed
        -0 checkstyle 0m 30s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 234 unchanged - 1 fixed = 235 total (was 235)
        +1 mvnsite 0m 56s the patch passed
        +1 mvneclipse 0m 11s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 2m 11s the patch passed
        +1 javadoc 0m 40s the patch passed
        +1 unit 74m 2s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 21s The patch does not generate ASF License warnings.
        102m 50s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11279
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845427/HDFS-11279.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux d84651fce929 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 591fb15
        Default Java 1.8.0_111
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18007/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18007/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18007/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 4 new or modified test files. +1 mvninstall 14m 48s trunk passed +1 compile 1m 1s trunk passed +1 checkstyle 0m 34s trunk passed +1 mvnsite 1m 3s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 2m 1s trunk passed +1 javadoc 0m 47s trunk passed +1 mvninstall 0m 55s the patch passed +1 compile 0m 51s the patch passed +1 javac 0m 51s the patch passed -0 checkstyle 0m 30s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 234 unchanged - 1 fixed = 235 total (was 235) +1 mvnsite 0m 56s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 11s the patch passed +1 javadoc 0m 40s the patch passed +1 unit 74m 2s hadoop-hdfs in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 102m 50s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11279 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845427/HDFS-11279.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d84651fce929 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 591fb15 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18007/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18007/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18007/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 13s Docker mode activated.
        +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.
        +1 mvninstall 13m 32s trunk passed
        +1 compile 0m 47s trunk passed
        +1 checkstyle 0m 30s trunk passed
        +1 mvnsite 0m 53s trunk passed
        +1 mvneclipse 0m 13s trunk passed
        +1 findbugs 1m 49s trunk passed
        +1 javadoc 0m 46s trunk passed
        +1 mvninstall 0m 49s the patch passed
        +1 compile 0m 45s the patch passed
        +1 javac 0m 45s the patch passed
        -0 checkstyle 0m 28s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 234 unchanged - 1 fixed = 235 total (was 235)
        +1 mvnsite 0m 51s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 57s the patch passed
        +1 javadoc 0m 38s the patch passed
        +1 unit 66m 11s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 22s The patch does not generate ASF License warnings.
        92m 14s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11279
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845433/HDFS-11279.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux a26552b1bf4f 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / ebdd2e0
        Default Java 1.8.0_111
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18008/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18008/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18008/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 13s Docker mode activated. +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. +1 mvninstall 13m 32s trunk passed +1 compile 0m 47s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 0m 53s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 49s trunk passed +1 javadoc 0m 46s trunk passed +1 mvninstall 0m 49s the patch passed +1 compile 0m 45s the patch passed +1 javac 0m 45s the patch passed -0 checkstyle 0m 28s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 234 unchanged - 1 fixed = 235 total (was 235) +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 57s the patch passed +1 javadoc 0m 38s the patch passed +1 unit 66m 11s hadoop-hdfs in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 92m 14s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11279 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845433/HDFS-11279.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a26552b1bf4f 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / ebdd2e0 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18008/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18008/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18008/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        xyao Xiaoyu Yao added a comment -

        Thanks Hanisha Koneru for updating the patch. Patch v002 looks good to me. I will commit it shortly.

        Show
        xyao Xiaoyu Yao added a comment - Thanks Hanisha Koneru for updating the patch. Patch v002 looks good to me. I will commit it shortly.
        Hide
        xyao Xiaoyu Yao added a comment -

        Thanks Hanisha Koneru for the contribution and all for the reviews. I've commit the v002 patch (with the . added at the end of the comments. for checkstyle issue in the last Jenkins) to the trunk.

        Show
        xyao Xiaoyu Yao added a comment - Thanks Hanisha Koneru for the contribution and all for the reviews. I've commit the v002 patch (with the . added at the end of the comments. for checkstyle issue in the last Jenkins) to the trunk.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11069 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11069/)
        HDFS-11279. Cleanup unused DataNode#checkDiskErrorAsync(). Contributed (xyao: rev 87bb1c49bb25f75b040028b1cebe3bc5251836d1)

        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailure.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/checker/DatasetVolumeChecker.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeHotSwapVolumes.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/checker/TestDatasetVolumeChecker.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11069 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11069/ ) HDFS-11279 . Cleanup unused DataNode#checkDiskErrorAsync(). Contributed (xyao: rev 87bb1c49bb25f75b040028b1cebe3bc5251836d1) (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailure.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/checker/DatasetVolumeChecker.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeHotSwapVolumes.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/checker/TestDatasetVolumeChecker.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Thank you Xiaoyu Yao for committing the patch.

        Show
        hanishakoneru Hanisha Koneru added a comment - Thank you Xiaoyu Yao for committing the patch.
        Hide
        arpitagarwal Arpit Agarwal added a comment -

        Pushed to branch-2.

        Show
        arpitagarwal Arpit Agarwal added a comment - Pushed to branch-2.

          People

          • Assignee:
            hanishakoneru Hanisha Koneru
            Reporter:
            xyao Xiaoyu Yao
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development