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

Log the name of the fsimage being loaded for better supportability

    Details

    • Hadoop Flags:
      Reviewed

      Description

      When NN starts to load fsimage, it does

       void loadFSImageFile(FSNamesystem target, MetaRecoveryContext recovery,
            FSImageFile imageFile, StartupOption startupOption) throws IOException {
            LOG.debug("Planning to load image :\n" + imageFile);
            ......
          long txId = loader.getLoadedImageTxId();
          LOG.info("Loaded image for txid " + txId + " from " + curFile);
      

      A debug msg is issued at the beginning with the fsimage file name, then at the end an info msg is issued after loading.

      If the fsimage loading failed due to corrupted fsimage (see HDFS-9406), we don't see the first msg. It'd be helpful to always be able to see from NN logs what fsimage file it's loading.

      Two improvements:

      1. Change the above debug to info
      2. If exception happens when loading fsimage, be sure to report the fsimage name being loaded in the error message.

      1. HDFS-9569.001.patch
        1 kB
        Yongjun Zhang
      2. HDFS-9569.002.patch
        3 kB
        Yongjun Zhang
      3. HDFS-9569.003.patch
        9 kB
        Yongjun Zhang
      4. HDFS-9569.004.patch
        10 kB
        Yongjun Zhang
      5. HDFS-9569.005.patch
        9 kB
        Yongjun Zhang

        Issue Links

          Activity

          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 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 mvninstall 7m 40s trunk passed
          +1 compile 0m 41s trunk passed with JDK v1.8.0_66
          +1 compile 0m 41s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 51s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 50s trunk passed
          +1 javadoc 1m 6s trunk passed with JDK v1.8.0_66
          +1 javadoc 1m 47s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 46s the patch passed
          +1 compile 0m 40s the patch passed with JDK v1.8.0_66
          +1 javac 0m 40s the patch passed
          +1 compile 0m 39s the patch passed with JDK v1.7.0_91
          +1 javac 0m 39s the patch passed
          +1 checkstyle 0m 15s the patch passed
          +1 mvnsite 0m 51s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 3s the patch passed
          +1 javadoc 1m 4s the patch passed with JDK v1.8.0_66
          +1 javadoc 1m 47s the patch passed with JDK v1.7.0_91
          -1 unit 51m 16s hadoop-hdfs in the patch failed with JDK v1.8.0_66.
          -1 unit 53m 28s hadoop-hdfs in the patch failed with JDK v1.7.0_91.
          -1 asflicense 0m 25s Patch generated 58 ASF License warnings.
          131m 7s



          Reason Tests
          JDK v1.8.0_66 Failed junit tests hadoop.hdfs.server.namenode.TestNNThroughputBenchmark
            hadoop.hdfs.TestDFSUpgradeFromImage
          JDK v1.7.0_91 Failed junit tests hadoop.hdfs.server.datanode.TestBlockReplacement
            hadoop.hdfs.TestDFSUpgradeFromImage
            hadoop.hdfs.TestAclsEndToEnd
            hadoop.hdfs.TestEncryptedTransfer



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12778168/HDFS-9569.001.patch
          JIRA Issue HDFS-9569
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 03208579956f 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f741476
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13910/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 75MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13910/console

          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 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 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 mvninstall 7m 40s trunk passed +1 compile 0m 41s trunk passed with JDK v1.8.0_66 +1 compile 0m 41s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 51s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 50s trunk passed +1 javadoc 1m 6s trunk passed with JDK v1.8.0_66 +1 javadoc 1m 47s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 46s the patch passed +1 compile 0m 40s the patch passed with JDK v1.8.0_66 +1 javac 0m 40s the patch passed +1 compile 0m 39s the patch passed with JDK v1.7.0_91 +1 javac 0m 39s the patch passed +1 checkstyle 0m 15s the patch passed +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 3s the patch passed +1 javadoc 1m 4s the patch passed with JDK v1.8.0_66 +1 javadoc 1m 47s the patch passed with JDK v1.7.0_91 -1 unit 51m 16s hadoop-hdfs in the patch failed with JDK v1.8.0_66. -1 unit 53m 28s hadoop-hdfs in the patch failed with JDK v1.7.0_91. -1 asflicense 0m 25s Patch generated 58 ASF License warnings. 131m 7s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.hdfs.server.namenode.TestNNThroughputBenchmark   hadoop.hdfs.TestDFSUpgradeFromImage JDK v1.7.0_91 Failed junit tests hadoop.hdfs.server.datanode.TestBlockReplacement   hadoop.hdfs.TestDFSUpgradeFromImage   hadoop.hdfs.TestAclsEndToEnd   hadoop.hdfs.TestEncryptedTransfer Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12778168/HDFS-9569.001.patch JIRA Issue HDFS-9569 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 03208579956f 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f741476 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13910/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/13910/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 75MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13910/console This message was automatically generated.
          Hide
          kihwal Kihwal Lee added a comment -

          +1 looks good to me.

          Show
          kihwal Kihwal Lee added a comment - +1 looks good to me.
          Hide
          kihwal Kihwal Lee added a comment -

          Committed this to trunk, branch-2, branch-2.8 and branch-2.7.

          Show
          kihwal Kihwal Lee added a comment - Committed this to trunk, branch-2, branch-2.8 and branch-2.7.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #8984 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8984/)
          HDFS-9569. Log the name of the fsimage being loaded for better (kihwal: rev eb6939cea0343840c62b930d4adb377f5eaf879f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8984 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8984/ ) HDFS-9569 . Log the name of the fsimage being loaded for better (kihwal: rev eb6939cea0343840c62b930d4adb377f5eaf879f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks a lot Kihwal Lee for the review and commit!

          Show
          yzhangal Yongjun Zhang added a comment - Thanks a lot Kihwal Lee for the review and commit!
          Hide
          cnauroth Chris Nauroth added a comment -

          I have reverted this patch from trunk, branch-2, branch-2.8 and branch-2.7. This patch introduced a test failure in TestDFSUpgradeFromImage#testUpgradeFromRel2ReservedImage. The test expects to see an IllegalArgumentException, and then retry the upgrade with the option to rename reserved paths. After this patch, the error handling masked the IllegalArgumentException, so the test no longer worked as expected.

          Show
          cnauroth Chris Nauroth added a comment - I have reverted this patch from trunk, branch-2, branch-2.8 and branch-2.7. This patch introduced a test failure in TestDFSUpgradeFromImage#testUpgradeFromRel2ReservedImage . The test expects to see an IllegalArgumentException , and then retry the upgrade with the option to rename reserved paths. After this patch, the error handling masked the IllegalArgumentException , so the test no longer worked as expected.
          Hide
          cnauroth Chris Nauroth added a comment -

          Pre-commit did flag the test failure in the last run too.

          Show
          cnauroth Chris Nauroth added a comment - Pre-commit did flag the test failure in the last run too.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #8989 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8989/)
          Revert "HDFS-9569. Log the name of the fsimage being loaded for better (cnauroth: rev db37f02dc704ad9eec8c56e3e466a5f37d138d74)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8989 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8989/ ) Revert " HDFS-9569 . Log the name of the fsimage being loaded for better (cnauroth: rev db37f02dc704ad9eec8c56e3e466a5f37d138d74) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks Chris for catching the test issue and reverting.

          Hi Kihwal Lee and Chris Nauroth,

          I just uploaded rev 2 to address the failure. Would you please help taking a look at the patch?

          Thanks.

          Show
          yzhangal Yongjun Zhang added a comment - Thanks Chris for catching the test issue and reverting. Hi Kihwal Lee and Chris Nauroth , I just uploaded rev 2 to address the failure. Would you please help taking a look at the patch? Thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 8m 29s trunk passed
          +1 compile 0m 48s trunk passed with JDK v1.8.0_66
          +1 compile 0m 44s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 57s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 2m 7s trunk passed
          +1 javadoc 1m 12s trunk passed with JDK v1.8.0_66
          +1 javadoc 2m 17s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 59s the patch passed
          +1 compile 0m 43s the patch passed with JDK v1.8.0_66
          -1 javac 7m 18s hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66 with JDK v1.8.0_66 generated 2 new issues (was 32, now 32).
          +1 javac 0m 43s the patch passed
          +1 compile 0m 45s the patch passed with JDK v1.7.0_91
          -1 javac 8m 3s hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91 with JDK v1.7.0_91 generated 2 new issues (was 34, now 34).
          +1 javac 0m 45s the patch passed
          +1 checkstyle 0m 16s the patch passed
          +1 mvnsite 0m 55s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 7s the patch passed
          +1 javadoc 1m 16s the patch passed with JDK v1.8.0_66
          +1 javadoc 2m 3s the patch passed with JDK v1.7.0_91
          -1 unit 55m 42s hadoop-hdfs in the patch failed with JDK v1.8.0_66.
          -1 unit 51m 42s hadoop-hdfs in the patch failed with JDK v1.7.0_91.
          +1 asflicense 0m 28s Patch does not generate ASF License warnings.
          136m 57s



          Reason Tests
          JDK v1.8.0_66 Failed junit tests hadoop.hdfs.server.namenode.TestNNThroughputBenchmark
            hadoop.hdfs.server.datanode.TestDataNodeMetrics
            hadoop.hdfs.server.datanode.TestBlockScanner
            hadoop.hdfs.qjournal.client.TestQuorumJournalManager
          JDK v1.7.0_91 Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes
            hadoop.hdfs.TestWriteReadStripedFile
            hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12778428/HDFS-9569.002.patch
          JIRA Issue HDFS-9569
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 13cc3ac4e031 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / bfadf11
          findbugs v3.0.0
          javac hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          javac hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91: https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13930/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 75MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13930/console

          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 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 29s trunk passed +1 compile 0m 48s trunk passed with JDK v1.8.0_66 +1 compile 0m 44s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 57s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 2m 7s trunk passed +1 javadoc 1m 12s trunk passed with JDK v1.8.0_66 +1 javadoc 2m 17s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 59s the patch passed +1 compile 0m 43s the patch passed with JDK v1.8.0_66 -1 javac 7m 18s hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66 with JDK v1.8.0_66 generated 2 new issues (was 32, now 32). +1 javac 0m 43s the patch passed +1 compile 0m 45s the patch passed with JDK v1.7.0_91 -1 javac 8m 3s hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91 with JDK v1.7.0_91 generated 2 new issues (was 34, now 34). +1 javac 0m 45s the patch passed +1 checkstyle 0m 16s the patch passed +1 mvnsite 0m 55s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 7s the patch passed +1 javadoc 1m 16s the patch passed with JDK v1.8.0_66 +1 javadoc 2m 3s the patch passed with JDK v1.7.0_91 -1 unit 55m 42s hadoop-hdfs in the patch failed with JDK v1.8.0_66. -1 unit 51m 42s hadoop-hdfs in the patch failed with JDK v1.7.0_91. +1 asflicense 0m 28s Patch does not generate ASF License warnings. 136m 57s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.hdfs.server.namenode.TestNNThroughputBenchmark   hadoop.hdfs.server.datanode.TestDataNodeMetrics   hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.qjournal.client.TestQuorumJournalManager JDK v1.7.0_91 Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes   hadoop.hdfs.TestWriteReadStripedFile   hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12778428/HDFS-9569.002.patch JIRA Issue HDFS-9569 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 13cc3ac4e031 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / bfadf11 findbugs v3.0.0 javac hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt javac hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91: https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HDFS-Build/13930/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13930/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 75MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13930/console This message was automatically generated.
          Hide
          yzhangal Yongjun Zhang added a comment -

          The test failure appears to be irrelevant to this change. Thanks.

          Show
          yzhangal Yongjun Zhang added a comment - The test failure appears to be irrelevant to this change. Thanks.
          Hide
          cnauroth Chris Nauroth added a comment -

          Patch v002 causes a subtle change in the control flow that will cause a lot of extraneous logging. For example, assume there are 3 fsimage directories, and they're all working correctly. With the current code, the first attempt to load finds reserved paths in the first directory and immediately exits by throwing an unchecked IllegalArgumentException. After applying the patch, this is converted to a checked IOException, which is not sufficient to terminate the loop. It will try all 3 fsimage directories. Each iteration will log an error, with full stack trace. Then, it will get logged a 4th time after the loop.

          This extra logging is not useful for end users. It's a better user experience to exit as soon as reserved names are encountered, because we already know at that point that user action is required.

          Maybe at this point we need a more specific exception type, like IllegalReservedPathException. That one could be allowed to propagate out of the loop and terminate early.

          Show
          cnauroth Chris Nauroth added a comment - Patch v002 causes a subtle change in the control flow that will cause a lot of extraneous logging. For example, assume there are 3 fsimage directories, and they're all working correctly. With the current code, the first attempt to load finds reserved paths in the first directory and immediately exits by throwing an unchecked IllegalArgumentException . After applying the patch, this is converted to a checked IOException , which is not sufficient to terminate the loop. It will try all 3 fsimage directories. Each iteration will log an error, with full stack trace. Then, it will get logged a 4th time after the loop. This extra logging is not useful for end users. It's a better user experience to exit as soon as reserved names are encountered, because we already know at that point that user action is required. Maybe at this point we need a more specific exception type, like IllegalReservedPathException . That one could be allowed to propagate out of the loop and terminate early.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Hi Chris Nauroth,

          Thanks for reviewing and good comments!

          Theoretically different fsimages could be corrupted different ways, so trying all and print error for individual fsimage might be ok. Printing the same error for each fsimage would not be a big problem IMO. But I agree that the reserved path exception tend to be the same for all fsimages to be tried, and we can keep the old control flow of throwing exception at the first fsimage tried.

          I uploaded rev 3 with your suggestion of introducing IllegalReservedPathException, which I think is a good idea.

          Would you please take a look?

          Thanks.

          Show
          yzhangal Yongjun Zhang added a comment - Hi Chris Nauroth , Thanks for reviewing and good comments! Theoretically different fsimages could be corrupted different ways, so trying all and print error for individual fsimage might be ok. Printing the same error for each fsimage would not be a big problem IMO. But I agree that the reserved path exception tend to be the same for all fsimages to be tried, and we can keep the old control flow of throwing exception at the first fsimage tried. I uploaded rev 3 with your suggestion of introducing IllegalReservedPathException, which I think is a good idea. Would you please take a look? Thanks.
          Hide
          cnauroth Chris Nauroth added a comment -

          Yongjun Zhang, thank you for the update. This looks better. It still ends up logging the error and full stack trace twice: once within the loadFSImage loop, and once again when the exception gets rethrown, escapes to top level, and exits the process. Sorry to nit-pick, but I think eliminating the redundant logging will better help operators identify what they need to do next. I suggest catching and rethrowing IllegalReservedPathException without logging from within the loop:

                  ...
                } catch (IllegalReservedPathException e) {
                  throw e;
                } catch (Exception e) {
                  LOG.error("Failed to load image from " + imageFile, e);
                  ...
          

          Also, the test can be simplified to catch IllegalReservedPathException directly:

                ...
              } catch (IllegalReservedPathException e) {
                GenericTestUtils.assertExceptionContains(
                    "reserved path component in this version",
                    e);
              } finally {
                ...
          

          I'll be +1 after those changes and another Jenkins run. I'll also give Kihwal Lee a chance to comment again before I commit anything, since he did the earlier review.

          Show
          cnauroth Chris Nauroth added a comment - Yongjun Zhang , thank you for the update. This looks better. It still ends up logging the error and full stack trace twice: once within the loadFSImage loop, and once again when the exception gets rethrown, escapes to top level, and exits the process. Sorry to nit-pick, but I think eliminating the redundant logging will better help operators identify what they need to do next. I suggest catching and rethrowing IllegalReservedPathException without logging from within the loop: ... } catch (IllegalReservedPathException e) { throw e; } catch (Exception e) { LOG.error( "Failed to load image from " + imageFile, e); ... Also, the test can be simplified to catch IllegalReservedPathException directly: ... } catch (IllegalReservedPathException e) { GenericTestUtils.assertExceptionContains( "reserved path component in this version" , e); } finally { ... I'll be +1 after those changes and another Jenkins run. I'll also give Kihwal Lee a chance to comment again before I commit anything, since he did the earlier review.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Hi Chris Nauroth,

          Thanks again for your review and comments.

          About your suggested change (option 1)

                } catch (IllegalReservedPathException e) {
                  throw e;
          

          The intention of my last change (option 2) was to include the fsimage file name in the exception, the above suggested change won't do it. That's why I did one error logging before rethrowing, so the logging has the imageFile name:

                } catch (IllegalReservedPathException e) {
                  LOG.error("Failed to load image from " + imageFile, e);
                  throw e;
          

          Another way to do it is (option 3):

                } catch (IllegalReservedPathException e) {
                    throw new IllegalReservedPathException("Failed to load image from "
                        + imageFile + " (" + e.getMessage() +")", e);
                  }
                  ...
          

          If we want to have the imageFile name in the exception message, which option would you prefer?

          Thanks.

          Show
          yzhangal Yongjun Zhang added a comment - Hi Chris Nauroth , Thanks again for your review and comments. About your suggested change (option 1) } catch (IllegalReservedPathException e) { throw e; The intention of my last change (option 2) was to include the fsimage file name in the exception, the above suggested change won't do it. That's why I did one error logging before rethrowing, so the logging has the imageFile name: } catch (IllegalReservedPathException e) { LOG.error( "Failed to load image from " + imageFile, e); throw e; Another way to do it is (option 3): } catch (IllegalReservedPathException e) { throw new IllegalReservedPathException( "Failed to load image from " + imageFile + " (" + e.getMessage() + ")" , e); } ... If we want to have the imageFile name in the exception message, which option would you prefer? Thanks.
          Hide
          cnauroth Chris Nauroth added a comment -

          How about this?

                } catch (IllegalReservedPathException e) {
                  LOG.error("Failed to load image from " + imageFile);
                  throw e;
          

          Notice this doesn't pass e to LOG.error. That way, we'll get a log message with the file name, but it won't double-print the stack trace.

          Show
          cnauroth Chris Nauroth added a comment - How about this? } catch (IllegalReservedPathException e) { LOG.error( "Failed to load image from " + imageFile); throw e; Notice this doesn't pass e to LOG.error . That way, we'll get a log message with the file name, but it won't double-print the stack trace.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 1s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 19m 28s trunk passed
          +1 compile 2m 42s trunk passed with JDK v1.8.0_66
          +1 compile 1m 55s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 41s trunk passed
          +1 mvnsite 2m 19s trunk passed
          +1 mvneclipse 0m 33s trunk passed
          +1 findbugs 4m 23s trunk passed
          +1 javadoc 3m 5s trunk passed with JDK v1.8.0_66
          +1 javadoc 4m 16s trunk passed with JDK v1.7.0_91
          +1 mvninstall 2m 17s the patch passed
          +1 compile 2m 37s the patch passed with JDK v1.8.0_66
          -1 javac 16m 42s hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66 with JDK v1.8.0_66 generated 2 new issues (was 32, now 32).
          +1 javac 2m 37s the patch passed
          +1 compile 1m 49s the patch passed with JDK v1.7.0_91
          -1 javac 18m 32s hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91 with JDK v1.7.0_91 generated 2 new issues (was 34, now 34).
          +1 javac 1m 49s the patch passed
          +1 checkstyle 0m 39s the patch passed
          +1 mvnsite 2m 17s the patch passed
          +1 mvneclipse 0m 31s the patch passed
          -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 findbugs 5m 8s the patch passed
          +1 javadoc 2m 58s the patch passed with JDK v1.8.0_66
          +1 javadoc 4m 18s the patch passed with JDK v1.7.0_91
          -1 unit 182m 53s hadoop-hdfs in the patch failed with JDK v1.8.0_66.
          -1 unit 177m 33s hadoop-hdfs in the patch failed with JDK v1.7.0_91.
          -1 asflicense 1m 6s Patch generated 1 ASF License warnings.
          431m 9s



          Reason Tests
          JDK v1.8.0_66 Failed junit tests hadoop.hdfs.TestDFSUpgradeFromImage
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure
            hadoop.hdfs.server.namenode.ha.TestBootstrapStandby
            hadoop.hdfs.server.datanode.TestBlockScanner
            hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency
            hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
            hadoop.hdfs.server.namenode.ha.TestEditLogTailer
            hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations
            hadoop.hdfs.security.TestDelegationTokenForProxyUser
            hadoop.hdfs.TestLocalDFS
            hadoop.hdfs.TestSafeModeWithStripedFile
            hadoop.hdfs.server.namenode.ha.TestStandbyCheckpoints
            hadoop.hdfs.tools.TestDFSAdminWithHA
            hadoop.hdfs.web.TestWebHdfsTokens
            hadoop.hdfs.TestDatanodeRegistration
            hadoop.hdfs.server.namenode.TestSecurityTokenEditLog
            hadoop.hdfs.server.datanode.TestBlockReplacement
            hadoop.hdfs.server.namenode.ha.TestSeveralNameNodes
            hadoop.hdfs.server.blockmanagement.TestRBWBlockInvalidation
            hadoop.hdfs.server.namenode.TestFileLimit
            hadoop.fs.TestSymlinkHdfsFileContext
            hadoop.hdfs.server.namenode.TestDecommissioningStatus
            hadoop.hdfs.qjournal.TestSecureNNWithQJM
            hadoop.hdfs.server.namenode.TestFileTruncate
            hadoop.hdfs.TestEncryptionZones
            hadoop.hdfs.server.namenode.TestCacheDirectives
            hadoop.hdfs.server.blockmanagement.TestComputeInvalidateWork
            hadoop.hdfs.server.datanode.TestDirectoryScanner
          JDK v1.7.0_91 Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure
            hadoop.hdfs.server.namenode.ha.TestBootstrapStandby
            hadoop.hdfs.server.datanode.TestBlockScanner
            hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency
            hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
            hadoop.hdfs.server.namenode.ha.TestEditLogTailer
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure060
            hadoop.hdfs.shortcircuit.TestShortCircuitCache
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure180
            hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations
            hadoop.hdfs.shortcircuit.TestShortCircuitLocalRead
            hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA
            hadoop.hdfs.security.TestDelegationTokenForProxyUser
            hadoop.hdfs.TestLocalDFS
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure170
            hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes
            hadoop.hdfs.web.TestWebHdfsTokens
            hadoop.hdfs.server.namenode.TestSecurityTokenEditLog
            hadoop.hdfs.server.datanode.TestBlockReplacement
            hadoop.fs.TestSymlinkHdfsFileContext
            hadoop.hdfs.server.namenode.TestDecommissioningStatus
            hadoop.hdfs.qjournal.TestSecureNNWithQJM
            hadoop.hdfs.server.namenode.TestFileTruncate
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure070
            hadoop.hdfs.TestEncryptionZones
            hadoop.hdfs.server.namenode.TestRecoverStripedBlocks
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure030
            hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery
            hadoop.hdfs.server.blockmanagement.TestComputeInvalidateWork
            hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot
            hadoop.hdfs.server.namenode.TestDeleteRace
            hadoop.hdfs.server.datanode.TestDirectoryScanner



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12778747/HDFS-9569.003.patch
          JIRA Issue HDFS-9569
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 600b83360f1b 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 7995a6e
          findbugs v3.0.0
          javac hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          javac hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91: https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13947/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 76MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13947/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 1s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 19m 28s trunk passed +1 compile 2m 42s trunk passed with JDK v1.8.0_66 +1 compile 1m 55s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 41s trunk passed +1 mvnsite 2m 19s trunk passed +1 mvneclipse 0m 33s trunk passed +1 findbugs 4m 23s trunk passed +1 javadoc 3m 5s trunk passed with JDK v1.8.0_66 +1 javadoc 4m 16s trunk passed with JDK v1.7.0_91 +1 mvninstall 2m 17s the patch passed +1 compile 2m 37s the patch passed with JDK v1.8.0_66 -1 javac 16m 42s hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66 with JDK v1.8.0_66 generated 2 new issues (was 32, now 32). +1 javac 2m 37s the patch passed +1 compile 1m 49s the patch passed with JDK v1.7.0_91 -1 javac 18m 32s hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91 with JDK v1.7.0_91 generated 2 new issues (was 34, now 34). +1 javac 1m 49s the patch passed +1 checkstyle 0m 39s the patch passed +1 mvnsite 2m 17s the patch passed +1 mvneclipse 0m 31s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 5m 8s the patch passed +1 javadoc 2m 58s the patch passed with JDK v1.8.0_66 +1 javadoc 4m 18s the patch passed with JDK v1.7.0_91 -1 unit 182m 53s hadoop-hdfs in the patch failed with JDK v1.8.0_66. -1 unit 177m 33s hadoop-hdfs in the patch failed with JDK v1.7.0_91. -1 asflicense 1m 6s Patch generated 1 ASF License warnings. 431m 9s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.hdfs.TestDFSUpgradeFromImage   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure   hadoop.hdfs.server.namenode.ha.TestBootstrapStandby   hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency   hadoop.hdfs.server.namenode.ha.TestPipelinesFailover   hadoop.hdfs.server.namenode.ha.TestEditLogTailer   hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations   hadoop.hdfs.security.TestDelegationTokenForProxyUser   hadoop.hdfs.TestLocalDFS   hadoop.hdfs.TestSafeModeWithStripedFile   hadoop.hdfs.server.namenode.ha.TestStandbyCheckpoints   hadoop.hdfs.tools.TestDFSAdminWithHA   hadoop.hdfs.web.TestWebHdfsTokens   hadoop.hdfs.TestDatanodeRegistration   hadoop.hdfs.server.namenode.TestSecurityTokenEditLog   hadoop.hdfs.server.datanode.TestBlockReplacement   hadoop.hdfs.server.namenode.ha.TestSeveralNameNodes   hadoop.hdfs.server.blockmanagement.TestRBWBlockInvalidation   hadoop.hdfs.server.namenode.TestFileLimit   hadoop.fs.TestSymlinkHdfsFileContext   hadoop.hdfs.server.namenode.TestDecommissioningStatus   hadoop.hdfs.qjournal.TestSecureNNWithQJM   hadoop.hdfs.server.namenode.TestFileTruncate   hadoop.hdfs.TestEncryptionZones   hadoop.hdfs.server.namenode.TestCacheDirectives   hadoop.hdfs.server.blockmanagement.TestComputeInvalidateWork   hadoop.hdfs.server.datanode.TestDirectoryScanner JDK v1.7.0_91 Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure   hadoop.hdfs.server.namenode.ha.TestBootstrapStandby   hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency   hadoop.hdfs.server.namenode.ha.TestPipelinesFailover   hadoop.hdfs.server.namenode.ha.TestEditLogTailer   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure060   hadoop.hdfs.shortcircuit.TestShortCircuitCache   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure180   hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations   hadoop.hdfs.shortcircuit.TestShortCircuitLocalRead   hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA   hadoop.hdfs.security.TestDelegationTokenForProxyUser   hadoop.hdfs.TestLocalDFS   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure170   hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes   hadoop.hdfs.web.TestWebHdfsTokens   hadoop.hdfs.server.namenode.TestSecurityTokenEditLog   hadoop.hdfs.server.datanode.TestBlockReplacement   hadoop.fs.TestSymlinkHdfsFileContext   hadoop.hdfs.server.namenode.TestDecommissioningStatus   hadoop.hdfs.qjournal.TestSecureNNWithQJM   hadoop.hdfs.server.namenode.TestFileTruncate   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure070   hadoop.hdfs.TestEncryptionZones   hadoop.hdfs.server.namenode.TestRecoverStripedBlocks   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure030   hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery   hadoop.hdfs.server.blockmanagement.TestComputeInvalidateWork   hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot   hadoop.hdfs.server.namenode.TestDeleteRace   hadoop.hdfs.server.datanode.TestDirectoryScanner Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12778747/HDFS-9569.003.patch JIRA Issue HDFS-9569 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 600b83360f1b 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 7995a6e findbugs v3.0.0 javac hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt javac hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91: https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13947/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/13947/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 76MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13947/console This message was automatically generated.
          Hide
          yzhangal Yongjun Zhang added a comment -

          HI Chris,

          I think your main concern at this point is not to log the trace stack twice, my main concern has been to ensure that the error issued shows both the trace stack and the file name. I think separating the error logging and exception throwing may not give clear cause-effect relationship.

          There are different reasons for failure to load fsimage. To be consistent, I think we can wrap the reason as a cause in an IOException. and let the code that catches the exception to find out the cause and deal with the cause accordingly.

          I'm uploading rev 4 that wraps cause in IOException and ensure the trace stack is printed only once. I wonder whether it will work for you.

          Thanks.

          Show
          yzhangal Yongjun Zhang added a comment - HI Chris, I think your main concern at this point is not to log the trace stack twice, my main concern has been to ensure that the error issued shows both the trace stack and the file name. I think separating the error logging and exception throwing may not give clear cause-effect relationship. There are different reasons for failure to load fsimage. To be consistent, I think we can wrap the reason as a cause in an IOException. and let the code that catches the exception to find out the cause and deal with the cause accordingly. I'm uploading rev 4 that wraps cause in IOException and ensure the trace stack is printed only once. I wonder whether it will work for you. Thanks.
          Hide
          cnauroth Chris Nauroth added a comment -

          OK, let's go with the rev 4 approach. However, I don't understand why this was added:

               List<FSImageFile> imageFiles = inspector.getLatestImages();
          -
          +    if (imageFiles.size() == 0) {
          +      throw new IOException("Failed to find any FSImage files.");
          +    }
          +    
          

          I think this is unreachable code. The implementations of getLatestImages are written to either return a non-empty list or throw an exception, so it appears to be impossible for the list to be empty by the time execution reaches here.

          Show
          cnauroth Chris Nauroth added a comment - OK, let's go with the rev 4 approach. However, I don't understand why this was added: List<FSImageFile> imageFiles = inspector.getLatestImages(); - + if (imageFiles.size() == 0) { + throw new IOException( "Failed to find any FSImage files." ); + } + I think this is unreachable code. The implementations of getLatestImages are written to either return a non-empty list or throw an exception, so it appears to be impossible for the list to be empty by the time execution reaches here.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks Chris Nauroth.

          I added the additional check thinking it was a missing one. Since getLatestImages ensures non-empty list is returned as you pointed out, I'm uploading rev 5 that has it removed. Nice catch of the new change I made.

          Show
          yzhangal Yongjun Zhang added a comment - Thanks Chris Nauroth . I added the additional check thinking it was a missing one. Since getLatestImages ensures non-empty list is returned as you pointed out, I'm uploading rev 5 that has it removed. Nice catch of the new change I made.
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for patch v005, pending Jenkins. Thank you!

          Show
          cnauroth Chris Nauroth added a comment - +1 for patch v005, pending Jenkins. Thank you!
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks Chris Nauroth!

          Show
          yzhangal Yongjun Zhang added a comment - Thanks Chris Nauroth !
          Hide
          cnauroth Chris Nauroth added a comment -

          Hi Yongjun Zhang. I had +1'd this. Were you planning to commit it, or would you like me to do it?

          Show
          cnauroth Chris Nauroth added a comment - Hi Yongjun Zhang . I had +1'd this. Were you planning to commit it, or would you like me to do it?
          Hide
          yzhangal Yongjun Zhang added a comment -

          HI Chris Nauroth, thanks for the reminder, I have been on vacation then spending time on a critical issue after getting back, will get this committed myself soon.

          Show
          yzhangal Yongjun Zhang added a comment - HI Chris Nauroth , thanks for the reminder, I have been on vacation then spending time on a critical issue after getting back, will get this committed myself soon.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9095 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9095/)
          HDFS-9569. Log the name of the fsimage being loaded for better (yzhang: rev 25051c3bd08efc12333a6acb51782cc7800403a4)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUpgradeFromImage.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/IllegalReservedPathException.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9095 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9095/ ) HDFS-9569 . Log the name of the fsimage being loaded for better (yzhang: rev 25051c3bd08efc12333a6acb51782cc7800403a4) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUpgradeFromImage.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/IllegalReservedPathException.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java
          Hide
          yzhangal Yongjun Zhang added a comment -

          I committed to trunk, branch-2, branch-2.8, and branch-2.7.

          Thanks Chris Nauroth and Kihwal Lee much for the review.

          Show
          yzhangal Yongjun Zhang added a comment - I committed to trunk, branch-2, branch-2.8, and branch-2.7. Thanks Chris Nauroth and Kihwal Lee much for the review.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          It seems that this caused TestStartup failing for the past 12 builds; see https://builds.apache.org/job/PreCommit-HDFS-Build/14117/testReport/org.apache.hadoop.hdfs.server.namenode/TestStartup/testImageChecksum/

          > +1 for patch v005, pending Jenkins. ...

          BTW, why there was the Jenkins report before committing?

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - It seems that this caused TestStartup failing for the past 12 builds; see https://builds.apache.org/job/PreCommit-HDFS-Build/14117/testReport/org.apache.hadoop.hdfs.server.namenode/TestStartup/testImageChecksum/ > +1 for patch v005, pending Jenkins. ... BTW, why there was the Jenkins report before committing?
          Hide
          yzhangal Yongjun Zhang added a comment -

          Hi Tsz Wo Nicholas Sze,

          Thanks for pointing out, the jenkins test was indeed overlooked, very sorry about the inconvenience, I will look into and try to resolve it today.

          Show
          yzhangal Yongjun Zhang added a comment - Hi Tsz Wo Nicholas Sze , Thanks for pointing out, the jenkins test was indeed overlooked, very sorry about the inconvenience, I will look into and try to resolve it today.
          Hide
          cnauroth Chris Nauroth added a comment -

          I missed it too. This patch lingered for a few weeks, and I mistakenly assumed Jenkins was done by now. Tsz Wo Nicholas Sze, thanks for pointing it out.

          Show
          cnauroth Chris Nauroth added a comment - I missed it too. This patch lingered for a few weeks, and I mistakenly assumed Jenkins was done by now. Tsz Wo Nicholas Sze , thanks for pointing it out.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Committed HDFS-9648 as the fix.

          Thanks Tsz Wo Nicholas Sze and Chris Nauroth.

          Show
          yzhangal Yongjun Zhang added a comment - Committed HDFS-9648 as the fix. Thanks Tsz Wo Nicholas Sze and Chris Nauroth .
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9114 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9114/)
          HDFS-9648. TestStartup.testImageChecksum is broken by HDFS-9569's (yzhang: rev 817cc1f02a60ef4e372171415058fdc76c0d2e39)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStartup.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9114 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9114/ ) HDFS-9648 . TestStartup.testImageChecksum is broken by HDFS-9569 's (yzhang: rev 817cc1f02a60ef4e372171415058fdc76c0d2e39) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStartup.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Thanks for the quick fix!

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Thanks for the quick fix!
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Closing the JIRA as part of 2.7.3 release.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Closing the JIRA as part of 2.7.3 release.

            People

            • Assignee:
              yzhangal Yongjun Zhang
              Reporter:
              yzhangal Yongjun Zhang
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development