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

BlockScanner is incorrectly disabled by default

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.7.0
    • Fix Version/s: 2.8.0, 2.7.1, 3.0.0-alpha1
    • Component/s: datanode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      This code is used to check whether the block scanner is enabled:

        public boolean isEnabled() {
          return (conf.scanPeriodMs) > 0 && (conf.targetBytesPerSec > 0);
        }
      

      Unfortunately, when this was introduced, we did not change the default scan period's value of 0, which means by default the BlockScanner is disabled.

      1. HDFS-8681.04.patch
        6 kB
        Arpit Agarwal
      2. HDFS-8681.03.patch
        5 kB
        Arpit Agarwal
      3. HDFS-8681.02.patch
        4 kB
        Arpit Agarwal
      4. HDFS-8681.01.patch
        0.8 kB
        Arpit Agarwal

        Issue Links

          Activity

          Hide
          arpitagarwal Arpit Agarwal added a comment - - edited

          Hi Andrew Wang does this mean the block scanner is not on by default in 2.7.x. If so it should be a blocker for 2.7.1.

          Show
          arpitagarwal Arpit Agarwal added a comment - - edited Hi Andrew Wang does this mean the block scanner is not on by default in 2.7.x. If so it should be a blocker for 2.7.1.
          Hide
          andrew.wang Andrew Wang added a comment -

          Sure, we can make it a blocker for 2.7.1. I didn't mark it as that originally since there's an easy workaround: set a value greater than 0 in the config.

          Ping Vinod Kumar Vavilapalli for heads up too.

          Show
          andrew.wang Andrew Wang added a comment - Sure, we can make it a blocker for 2.7.1. I didn't mark it as that originally since there's an easy workaround: set a value greater than 0 in the config. Ping Vinod Kumar Vavilapalli for heads up too.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          we did not change the default scan period's value of 0,

          Instead of changing the default can we just change the check to be conf.scanPeriodMs >= 0? Is there any part of HDFS-7430 that would be broken if the scanPeriod is 0?

          Show
          arpitagarwal Arpit Agarwal added a comment - we did not change the default scan period's value of 0, Instead of changing the default can we just change the check to be conf.scanPeriodMs >= 0 ? Is there any part of HDFS-7430 that would be broken if the scanPeriod is 0?
          Hide
          cnauroth Chris Nauroth added a comment -

          The old logic, pre-HDFS-7430, was as Arpit described, disabling only for a negative value.

            private synchronized void initDataBlockScanner(Configuration conf) {
              if (blockScanner != null) {
                return;
              }
              String reason = null;
              assert data != null;
              if (conf.getInt(DFS_DATANODE_SCAN_PERIOD_HOURS_KEY,
                              DFS_DATANODE_SCAN_PERIOD_HOURS_DEFAULT) < 0) {
                reason = "verification is turned off by configuration";
              } else if ("SimulatedFSDataset".equals(data.getClass().getSimpleName())) {
                reason = "verifcation is not supported by SimulatedFSDataset";
              } 
              if (reason == null) {
                blockScanner = new DataBlockScanner(this, data, conf);
                blockScanner.start();
              } else {
                LOG.info("Periodic Block Verification scan disabled because " + reason);
              }
            }
          

          Backwards-compatibility requires that we restore that logic. Otherwise, if someone had a hdfs-site.xml file that set dfs.datanode.scan.period.hours to 0 explicitly, then it would stop working.

          Show
          cnauroth Chris Nauroth added a comment - The old logic, pre- HDFS-7430 , was as Arpit described, disabling only for a negative value. private synchronized void initDataBlockScanner(Configuration conf) { if (blockScanner != null ) { return ; } String reason = null ; assert data != null ; if (conf.getInt(DFS_DATANODE_SCAN_PERIOD_HOURS_KEY, DFS_DATANODE_SCAN_PERIOD_HOURS_DEFAULT) < 0) { reason = "verification is turned off by configuration" ; } else if ( "SimulatedFSDataset" .equals(data.getClass().getSimpleName())) { reason = "verifcation is not supported by SimulatedFSDataset" ; } if (reason == null ) { blockScanner = new DataBlockScanner( this , data, conf); blockScanner.start(); } else { LOG.info( "Periodic Block Verification scan disabled because " + reason); } } Backwards-compatibility requires that we restore that logic. Otherwise, if someone had a hdfs-site.xml file that set dfs.datanode.scan.period.hours to 0 explicitly, then it would stop working.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Patch to treat scanPeriodMs of zero as enabled.

          Show
          arpitagarwal Arpit Agarwal added a comment - Patch to treat scanPeriodMs of zero as enabled.
          Hide
          arpitagarwal Arpit Agarwal added a comment - - edited

          On further look here is the code from branch-2.6

              long hours = conf.getInt(DFSConfigKeys.DFS_DATANODE_SCAN_PERIOD_HOURS_KEY, 
                                       DFSConfigKeys.DFS_DATANODE_SCAN_PERIOD_HOURS_DEFAULT);
              if (hours <= 0) {
                hours = DEFAULT_SCAN_PERIOD_HOURS;
              }
          

          where

            private static final long DEFAULT_SCAN_PERIOD_HOURS = 21*24L; // three weeks
          

          Looking into whether we can just restore this fallback to 3 weeks if the value is un-configured or set to zero.

          Show
          arpitagarwal Arpit Agarwal added a comment - - edited On further look here is the code from branch-2.6 long hours = conf.getInt(DFSConfigKeys.DFS_DATANODE_SCAN_PERIOD_HOURS_KEY, DFSConfigKeys.DFS_DATANODE_SCAN_PERIOD_HOURS_DEFAULT); if (hours <= 0) { hours = DEFAULT_SCAN_PERIOD_HOURS; } where private static final long DEFAULT_SCAN_PERIOD_HOURS = 21*24L; // three weeks Looking into whether we can just restore this fallback to 3 weeks if the value is un-configured or set to zero.
          Hide
          jnp Jitendra Nath Pandey added a comment -

          I am +1 for restoring the behavior of branch-2.6. There is no point in scanning the block pools more than once in several days.

          Show
          jnp Jitendra Nath Pandey added a comment - I am +1 for restoring the behavior of branch-2.6. There is no point in scanning the block pools more than once in several days.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          v2 patch attempts to retain compatibility with the quirks of the old behavior. If the value is not configured or set to <= 0, then fallback to 3 weeks. I've changed the default in DFSConfigKeys from 0 to 3 weeks, so we don't use the internal 'magic' value DEFAULT_SCAN_PERIOD_HOURS.

          I'd appreciate a careful review since this is close to 2.7.1 release.

          Show
          arpitagarwal Arpit Agarwal added a comment - v2 patch attempts to retain compatibility with the quirks of the old behavior. If the value is not configured or set to <= 0, then fallback to 3 weeks. I've changed the default in DFSConfigKeys from 0 to 3 weeks, so we don't use the internal 'magic' value DEFAULT_SCAN_PERIOD_HOURS. I'd appreciate a careful review since this is close to 2.7.1 release.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Also update documentation.

          Show
          arpitagarwal Arpit Agarwal added a comment - Also update documentation.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Update documentation and fix behavior with negative values to be consistent with 2.6.

          See updated hdfs-default.xml for the contract.

          Show
          arpitagarwal Arpit Agarwal added a comment - Update documentation and fix behavior with negative values to be consistent with 2.6. See updated hdfs-default.xml for the contract.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 17m 52s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 31s There were no new javac warning messages.
          +1 javadoc 9m 40s There were no new javadoc warning messages.
          +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
          +1 checkstyle 2m 13s There were no new checkstyle issues.
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 32s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 3m 16s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 native 3m 15s Pre-build of native portion
          -1 hdfs tests 165m 16s Tests failed in hadoop-hdfs.
              211m 34s  



          Reason Tests
          Failed unit tests hadoop.hdfs.TestClientReportBadBlock
            hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaPlacement
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
            hadoop.hdfs.server.datanode.fsdataset.impl.TestScrLazyPersistFiles
            hadoop.hdfs.server.datanode.TestCachingStrategy
            hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyWriter



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12742372/HDFS-8681.01.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 79ed0f9
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/11518/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11518/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11518/testReport/
          Java 1.7.0_55
          uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11518/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 17m 52s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 31s There were no new javac warning messages. +1 javadoc 9m 40s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 2m 13s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 32s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 3m 16s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 15s Pre-build of native portion -1 hdfs tests 165m 16s Tests failed in hadoop-hdfs.     211m 34s   Reason Tests Failed unit tests hadoop.hdfs.TestClientReportBadBlock   hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaPlacement   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl   hadoop.hdfs.server.datanode.fsdataset.impl.TestScrLazyPersistFiles   hadoop.hdfs.server.datanode.TestCachingStrategy   hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyWriter Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12742372/HDFS-8681.01.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 79ed0f9 Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/11518/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11518/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11518/testReport/ Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11518/console This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 18m 0s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 32s There were no new javac warning messages.
          +1 javadoc 9m 40s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 2m 17s The applied patch generated 1 new checkstyle issues (total was 427, now 428).
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 35s mvn install still works.
          +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
          +1 findbugs 3m 17s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 native 3m 14s Pre-build of native portion
          -1 hdfs tests 162m 25s Tests failed in hadoop-hdfs.
              208m 59s  



          Reason Tests
          Failed unit tests hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12742374/HDFS-8681.02.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / b543d1a
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/11519/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11519/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11519/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11519/testReport/
          Java 1.7.0_55
          uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11519/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 18m 0s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 32s There were no new javac warning messages. +1 javadoc 9m 40s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 2m 17s The applied patch generated 1 new checkstyle issues (total was 427, now 428). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 35s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. +1 findbugs 3m 17s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 14s Pre-build of native portion -1 hdfs tests 162m 25s Tests failed in hadoop-hdfs.     208m 59s   Reason Tests Failed unit tests hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12742374/HDFS-8681.02.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / b543d1a Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/11519/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11519/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11519/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11519/testReport/ Java 1.7.0_55 uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11519/console This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 18m 4s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 34s There were no new javac warning messages.
          +1 javadoc 9m 39s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 2m 22s The applied patch generated 1 new checkstyle issues (total was 427, now 428).
          -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 install 1m 35s mvn install still works.
          +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
          +1 findbugs 3m 20s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 native 3m 14s Pre-build of native portion
          -1 hdfs tests 163m 5s Tests failed in hadoop-hdfs.
              209m 52s  



          Reason Tests
          Failed unit tests hadoop.hdfs.server.namenode.ha.TestStandbyIsHot
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12742376/HDFS-8681.03.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / b543d1a
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/11520/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11520/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt
          whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/11520/artifact/patchprocess/whitespace.txt
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11520/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11520/testReport/
          Java 1.7.0_55
          uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11520/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 18m 4s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 34s There were no new javac warning messages. +1 javadoc 9m 39s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 2m 22s The applied patch generated 1 new checkstyle issues (total was 427, now 428). -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 35s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. +1 findbugs 3m 20s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 14s Pre-build of native portion -1 hdfs tests 163m 5s Tests failed in hadoop-hdfs.     209m 52s   Reason Tests Failed unit tests hadoop.hdfs.server.namenode.ha.TestStandbyIsHot   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12742376/HDFS-8681.03.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / b543d1a Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/11520/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11520/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/11520/artifact/patchprocess/whitespace.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11520/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11520/testReport/ Java 1.7.0_55 uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11520/console This message was automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Colin does not seem working on this. Assigning to Arpit.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Colin does not seem working on this. Assigning to Arpit.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          (We should remove getUnitTestLong(..) and BlockScanner.Conf.INTERNAL_XXX fields since they unnecessarily complicate the main code with the testing code. Instead, we could simply add a BlockScanner(DataNode, Conf) for the unit tests. Let's do it separately.)

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - +1 patch looks good. (We should remove getUnitTestLong(..) and BlockScanner.Conf.INTERNAL_XXX fields since they unnecessarily complicate the main code with the testing code. Instead, we could simply add a BlockScanner(DataNode, Conf) for the unit tests. Let's do it separately.)
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          The failure of TestFsDatasetImpl seems related to the patch. Please take a look.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - The failure of TestFsDatasetImpl seems related to the patch. Please take a look.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          I suggest to just disable BlockScanner for the mockito test as below since it is the behavior before the patch and testChangeVolumeWithRunningCheckDirs() is not testing BlockScanner.

          +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          @@ -274,6 +274,7 @@ public void testRemoveNewlyAddedVolume() throws IOException {
             public void testChangeVolumeWithRunningCheckDirs() throws IOException {
               RoundRobinVolumeChoosingPolicy<FsVolumeImpl> blockChooser =
                   new RoundRobinVolumeChoosingPolicy<>();
          +    conf.setLong(DFSConfigKeys.DFS_DATANODE_SCAN_PERIOD_HOURS_KEY, -1);
               final BlockScanner blockScanner = new BlockScanner(datanode, conf);
               final FsVolumeList volumeList = new FsVolumeList(
                   Collections.<VolumeFailureInfo>emptyList(), blockScanner, blockChooser);
          
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - I suggest to just disable BlockScanner for the mockito test as below since it is the behavior before the patch and testChangeVolumeWithRunningCheckDirs() is not testing BlockScanner. +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java @@ -274,6 +274,7 @@ public void testRemoveNewlyAddedVolume() throws IOException { public void testChangeVolumeWithRunningCheckDirs() throws IOException { RoundRobinVolumeChoosingPolicy<FsVolumeImpl> blockChooser = new RoundRobinVolumeChoosingPolicy<>(); + conf.setLong(DFSConfigKeys.DFS_DATANODE_SCAN_PERIOD_HOURS_KEY, -1); final BlockScanner blockScanner = new BlockScanner(datanode, conf); final FsVolumeList volumeList = new FsVolumeList( Collections.<VolumeFailureInfo>emptyList(), blockScanner, blockChooser);
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Thanks for the review and feedback Tsz Wo Nicholas Sze.

          Updated patch fixes the test case. I agree about the test code in the class, will file a separate Jira to fix it.

          Show
          arpitagarwal Arpit Agarwal added a comment - Thanks for the review and feedback Tsz Wo Nicholas Sze . Updated patch fixes the test case. I agree about the test code in the class, will file a separate Jira to fix it.
          Hide
          jnp Jitendra Nath Pandey added a comment -

          +1 for the latest patch.

          Show
          jnp Jitendra Nath Pandey added a comment - +1 for the latest patch.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 17m 46s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
          +1 javac 7m 30s There were no new javac warning messages.
          +1 javadoc 9m 34s There were no new javadoc warning messages.
          +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 2m 16s The applied patch generated 1 new checkstyle issues (total was 427, now 428).
          -1 whitespace 0m 1s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 install 1m 33s mvn install still works.
          +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
          +1 findbugs 3m 20s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 native 3m 15s Pre-build of native portion
          -1 hdfs tests 162m 4s Tests failed in hadoop-hdfs.
              208m 18s  



          Reason Tests
          Failed unit tests hadoop.hdfs.server.namenode.TestFileTruncate



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12742399/HDFS-8681.04.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / b543d1a
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/11521/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11521/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt
          whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/11521/artifact/patchprocess/whitespace.txt
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11521/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11521/testReport/
          Java 1.7.0_55
          uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11521/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 17m 46s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 30s There were no new javac warning messages. +1 javadoc 9m 34s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 2m 16s The applied patch generated 1 new checkstyle issues (total was 427, now 428). -1 whitespace 0m 1s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 33s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 findbugs 3m 20s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 15s Pre-build of native portion -1 hdfs tests 162m 4s Tests failed in hadoop-hdfs.     208m 18s   Reason Tests Failed unit tests hadoop.hdfs.server.namenode.TestFileTruncate Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12742399/HDFS-8681.04.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / b543d1a Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/11521/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11521/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/11521/artifact/patchprocess/whitespace.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11521/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11521/testReport/ Java 1.7.0_55 uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11521/console This message was automatically generated.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          The test failure looks unrelated and I fixed the whitespace issue on commit. The checkstyle issue can be ignored, adjacent lines in the same file are all over 80 characters. Perhaps we can clean up the file in another commit.

          Thanks for the reviews Tsz Wo Nicholas Sze and Jitendra Nath Pandey. Committed for 2.7. Also thanks for reporting this Andrew Wang, good catch.

          Show
          arpitagarwal Arpit Agarwal added a comment - The test failure looks unrelated and I fixed the whitespace issue on commit. The checkstyle issue can be ignored, adjacent lines in the same file are all over 80 characters. Perhaps we can clean up the file in another commit. Thanks for the reviews Tsz Wo Nicholas Sze and Jitendra Nath Pandey . Committed for 2.7. Also thanks for reporting this Andrew Wang , good catch.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #8080 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8080/)
          HDFS-8681. BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8080 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8080/ ) HDFS-8681 . BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #243 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/243/)
          HDFS-8681. BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #243 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/243/ ) HDFS-8681 . BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #973 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/973/)
          HDFS-8681. BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #973 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/973/ ) HDFS-8681 . BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2171 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2171/)
          HDFS-8681. BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2171 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2171/ ) HDFS-8681 . BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #232 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/232/)
          HDFS-8681. BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #232 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/232/ ) HDFS-8681 . BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2189 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2189/)
          HDFS-8681. BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2189 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2189/ ) HDFS-8681 . BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #241 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/241/)
          HDFS-8681. BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #241 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/241/ ) HDFS-8681 . BlockScanner is incorrectly disabled by default. (Contributed by Arpit Agarwal) (arp: rev c6793dd8cc69ea994eb23c3e1349efe4b9feca9a) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockScanner.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Filed HDFS-8691 to simplify initialization as suggested by Nicholas and add more tests to verify contract.

          Show
          arpitagarwal Arpit Agarwal added a comment - Filed HDFS-8691 to simplify initialization as suggested by Nicholas and add more tests to verify contract.
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks for the quick action here all, patch LGTM too.

          Show
          andrew.wang Andrew Wang added a comment - Thanks for the quick action here all, patch LGTM too.
          Hide
          cmccabe Colin P. McCabe added a comment -

          +1. Thanks, guys.

          Show
          cmccabe Colin P. McCabe added a comment - +1. Thanks, guys.

            People

            • Assignee:
              arpitagarwal Arpit Agarwal
              Reporter:
              andrew.wang Andrew Wang
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development