Hadoop Common
  1. Hadoop Common
  2. HADOOP-5193

SecondaryNameNode does not rollImage because of incorrect calculation of edits modification time.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.19.0
    • Fix Version/s: 0.19.1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Secondary name-node cannot complete the second phase of the checkpoint because getFsEditsTime() returns the mod time of edits.new rather than edits file.
      The difference is that edits remains unchanged during the whole checkpoint process an therefore can serve as an invariant. On the contrary edits.new is changing all the time since it is the target of the edits log during checkpoint. So comparison of the mod time of edits.new before and after checkpoint fail and name-node does not upload new image file from the secondary node and does not truncate edits files.

      1. EditsLength.patch
        2 kB
        Konstantin Shvachko

        Activity

        Hide
        Konstantin Shvachko added a comment -

        This changes calculation of the edits length.
        I also removed length() method from{{ EditLogOutputStream}}, because this was the only use case for it.

        Show
        Konstantin Shvachko added a comment - This changes calculation of the edits length. I also removed length() method from{{ EditLogOutputStream}}, because this was the only use case for it.
        Hide
        Boris Shkolnik added a comment -

        +1, looks good

        note. you actually removed lastModified() and not length().

        Show
        Boris Shkolnik added a comment - +1, looks good note. you actually removed lastModified() and not length().
        Hide
        Konstantin Shvachko added a comment -

        Yes I meant lastModified() not length(). Thanks.

        Show
        Konstantin Shvachko added a comment - Yes I meant lastModified() not length(). Thanks.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12399671/EditsLength.patch
        against trunk revision 741762.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no tests are needed for this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        +1 Eclipse classpath. The patch retains Eclipse classpath integrity.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        -1 contrib tests. The patch failed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3809/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3809/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3809/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3809/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12399671/EditsLength.patch against trunk revision 741762. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no tests are needed for this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3809/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3809/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3809/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3809/console This message is automatically generated.
        Hide
        Konstantin Shvachko added a comment -

        TestAgentConfig.testInitAdaptors_vs_Checkpoint fails because of HADOOP-5172.
        TestTaskLimits timed out. Probably one of those cases that some previous test did not stop some servers.
        Anyway it passes in my own builds and it has nothing to do with SecondaryNameNode at all.

        Show
        Konstantin Shvachko added a comment - TestAgentConfig.testInitAdaptors_vs_Checkpoint fails because of HADOOP-5172 . TestTaskLimits timed out. Probably one of those cases that some previous test did not stop some servers. Anyway it passes in my own builds and it has nothing to do with SecondaryNameNode at all.
        Hide
        Konstantin Shvachko added a comment -

        I just committed this.

        Show
        Konstantin Shvachko added a comment - I just committed this.
        Hide
        Nigel Daley added a comment -

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no tests are needed for this patch.

        Konstantin, why no regression test?

        Boris, why didn't you ask about this in the code review? Our checklist is here: http://wiki.apache.org/hadoop/CodeReviewChecklist

        Show
        Nigel Daley added a comment - -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no tests are needed for this patch. Konstantin, why no regression test? Boris, why didn't you ask about this in the code review? Our checklist is here: http://wiki.apache.org/hadoop/CodeReviewChecklist
        Hide
        Hudson added a comment -
        Show
        Hudson added a comment - Integrated in Hadoop-trunk #756 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/756/ )

          People

          • Assignee:
            Konstantin Shvachko
            Reporter:
            Konstantin Shvachko
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development