Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.22.0, 0.23.0
    • Fix Version/s: 0.22.0, 0.23.0
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Will attach the full log. Here's the relevant snippet:

      Exception in thread "FSImageSaver for /home/eli/src/hdfs4/build/test/data/dfs/na
      me1 of type IMAGE_AND_EDITS" java.lang.RuntimeException: Injected fault: saveFSI
      mage second time
      ....
              at java.lang.Thread.run(Thread.java:619)
      Exception in thread "FSImageSaver for /home/eli/src/hdfs4/build/test/data/dfs/na
      me2 of type IMAGE_AND_EDITS" java.lang.StackOverflowError
              at java.util.Arrays.copyOf(Arrays.java:2882)
      
      1. TEST-org.apache.hadoop.hdfs.server.namenode.TestSaveNamespace.txt
        817 kB
        Eli Collins
      2. hdfs-1503.txt
        1 kB
        Todd Lipcon
      3. hdfs-1503_0.22.patch
        1 kB
        Konstantin Boudnik

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          12d 9h 55m 1 Konstantin Boudnik 30/Nov/10 05:43
          Resolved Resolved Closed Closed
          3m 42s 1 Eli Collins 30/Nov/10 05:47
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-22-branch #35 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-22-branch/35/)

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-22-branch #35 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-22-branch/35/ )
          Hide
          Konstantin Boudnik added a comment -

          Please disregard my last comment about 0.22 - looks like Eli has merged the commit to 0.22 almost concurrently, although it seemed unnecessary.

          Show
          Konstantin Boudnik added a comment - Please disregard my last comment about 0.22 - looks like Eli has merged the commit to 0.22 almost concurrently, although it seemed unnecessary.
          Eli Collins made changes -
          Fix Version/s 0.22.0 [ 12314241 ]
          Eli Collins made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Eli Collins added a comment -

          Hey Cos,

          Looks like we were working at the same time, I already backported it to 22 (which is why you probably saw that the patch didn't apply).

          Thanks,
          Eli

          Show
          Eli Collins added a comment - Hey Cos, Looks like we were working at the same time, I already backported it to 22 (which is why you probably saw that the patch didn't apply). Thanks, Eli
          Konstantin Boudnik made changes -
          Attachment hdfs-1503_0.22.patch [ 12464941 ]
          Hide
          Konstantin Boudnik added a comment -

          Just in case the modification of the patch for 0.22 in case it needs to be committed as well.

          Show
          Konstantin Boudnik added a comment - Just in case the modification of the patch for 0.22 in case it needs to be committed as well.
          Konstantin Boudnik made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 0.23.0 [ 12315571 ]
          Resolution Fixed [ 1 ]
          Hide
          Konstantin Boudnik added a comment -

          I have just committed this to trunk. Thanks Todd!

          The patch doesn't apply to 0.22. Moreover, in 0.22 this test passes on my machine. Please let me know if this patch (with minor variation) is applicable for 0.22 as well.

          Show
          Konstantin Boudnik added a comment - I have just committed this to trunk. Thanks Todd! The patch doesn't apply to 0.22. Moreover, in 0.22 this test passes on my machine. Please let me know if this patch (with minor variation) is applicable for 0.22 as well.
          Eli Collins made changes -
          Hadoop Flags [Reviewed]
          Affects Version/s 0.22.0 [ 12314241 ]
          Affects Version/s 0.23.0 [ 12315571 ]
          Hide
          Eli Collins added a comment -
               [exec] 
               [exec] +1 overall.  
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     +1 tests included.  The patch appears to include 3 new or modified tests.
               [exec] 
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
               [exec] 
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
               [exec] 
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs (version 1.3.9) warnings.
               [exec] 
               [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
               [exec] 
               [exec]     +1 system test framework.  The patch passed system test framework compile.
               [exec] 
          
          Show
          Eli Collins added a comment - [exec] [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system test framework. The patch passed system test framework compile. [exec]
          Hide
          Todd Lipcon added a comment -

          anyone want to commit?

          Show
          Todd Lipcon added a comment - anyone want to commit?
          Hide
          Konstantin Shvachko added a comment -

          +1. Tested on my box: testCrashWhileSavingSecondImage() fails without the patch, but passes with it.

          Show
          Konstantin Shvachko added a comment - +1. Tested on my box: testCrashWhileSavingSecondImage() fails without the patch, but passes with it.
          Hide
          Konstantin Shvachko added a comment -

          Thanks for clarifications, Todd. I was not sure if the new code broke the test itself or the feature it is testing. Now I understand it the test problem.

          Show
          Konstantin Shvachko added a comment - Thanks for clarifications, Todd. I was not sure if the new code broke the test itself or the feature it is testing. Now I understand it the test problem.
          Hide
          Todd Lipcon added a comment -

          Sure, no problem. This test uses a mockito "spy" object to inject some code right before the saveFSImage call. In some cases we want to call through to the original saveFSImage, in other cases we just want to throw an exception. For whatever reason, an incorrect change was made in HDFS-1071 which made the "call through" case call through to the same spy object rather than the unmodified class, so of course it infinitely recursed and blew out the stack. The patch switches to using the more "proper" way of doing this in Mockito - the callRealMethod() function.

          Show
          Todd Lipcon added a comment - Sure, no problem. This test uses a mockito "spy" object to inject some code right before the saveFSImage call. In some cases we want to call through to the original saveFSImage, in other cases we just want to throw an exception. For whatever reason, an incorrect change was made in HDFS-1071 which made the "call through" case call through to the same spy object rather than the unmodified class, so of course it infinitely recursed and blew out the stack. The patch switches to using the more "proper" way of doing this in Mockito - the callRealMethod() function.
          Hide
          Konstantin Shvachko added a comment -

          Could anybody please explain the problem and the solution. A don't understand it from the comments above.

          Show
          Konstantin Shvachko added a comment - Could anybody please explain the problem and the solution. A don't understand it from the comments above.
          Hide
          Konstantin Boudnik added a comment -

          +1 patch looks good.

          Show
          Konstantin Boudnik added a comment - +1 patch looks good.
          Jakob Homan made changes -
          Assignee Jakob Homan [ jghoman ] Todd Lipcon [ tlipcon ]
          Todd Lipcon made changes -
          Attachment hdfs-1503.txt [ 12459824 ]
          Hide
          Todd Lipcon added a comment -

          This patch fixes it for me.

          Show
          Todd Lipcon added a comment - This patch fixes it for me.
          Hide
          Todd Lipcon added a comment -

          Seems the first problem is that the faulty image delegates back to itself rather than the original image, so you get a stack overflow. After fixing that, the test still fails.

          Show
          Todd Lipcon added a comment - Seems the first problem is that the faulty image delegates back to itself rather than the original image, so you get a stack overflow. After fixing that, the test still fails.
          Jakob Homan made changes -
          Assignee Jakob Homan [ jghoman ]
          Todd Lipcon made changes -
          Summary TestSaveNamespace fails when FI is enabled TestSaveNamespace fails
          Hide
          Todd Lipcon added a comment -

          edited issue summary - this isn't from one of the AOP-based FI tests - it's just fault injection using Mockito from a normal unit test

          Show
          Todd Lipcon added a comment - edited issue summary - this isn't from one of the AOP-based FI tests - it's just fault injection using Mockito from a normal unit test
          Hide
          Todd Lipcon added a comment -

          This is a regression caused by HDFS-1071

          Show
          Todd Lipcon added a comment - This is a regression caused by HDFS-1071
          Todd Lipcon made changes -
          Link This issue is broken by HDFS-1071 [ HDFS-1071 ]
          Eli Collins made changes -
          Field Original Value New Value
          Attachment TEST-org.apache.hadoop.hdfs.server.namenode.TestSaveNamespace.txt [ 12459822 ]
          Hide
          Eli Collins added a comment -

          Full log attached.

          Show
          Eli Collins added a comment - Full log attached.
          Eli Collins created issue -

            People

            • Assignee:
              Todd Lipcon
              Reporter:
              Eli Collins
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development