Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Given that 0.13 is incredibly old at this point, I think the likelihood of anyone trying to start an 0.20+ namenode on an 0.13 directly layout is essentially nil. The intentionally-corrupt "$

      {dfs.name.dir}

      /image/" directory just serves to confuse new users at this point, so I propose removing it.

      If no one objects, I'll submit a patch. If there are objections, I propose at least adding a README explaining its purpose (same text as in the corrupt fsimage file)

      1. hdfs-259.txt
        8 kB
        Todd Lipcon
      2. hdfs-259.txt
        9 kB
        Todd Lipcon
      3. hdfs-259.txt
        12 kB
        Todd Lipcon
      4. hdfs-259.txt
        12 kB
        Eli Collins
      5. hdfs-259.txt
        13 kB
        Todd Lipcon
      6. hdfs-259.txt
        14 kB
        Todd Lipcon

        Activity

        Todd Lipcon created issue -
        Hide
        dhruba borthakur added a comment -

        The disk layout version is different from the release version.

        For code maintainability purposes, we should deprecate log versions that occured 5 release back seems like a good option. What log version was there for release 0.13?

        Show
        dhruba borthakur added a comment - The disk layout version is different from the release version. For code maintainability purposes, we should deprecate log versions that occured 5 release back seems like a good option. What log version was there for release 0.13?
        Hide
        Todd Lipcon added a comment -

        I may be mistaken, but I believe the $

        {dfs.name.dir}

        /image/ directory is from version 0.13 rather than layout 13, though I'd have to check svn logs to be sure. Either way, does anyone honestly believe that an in-place upgrade will work for versions prior to 0.16 or so in practice?

        Show
        Todd Lipcon added a comment - I may be mistaken, but I believe the $ {dfs.name.dir} /image/ directory is from version 0.13 rather than layout 13, though I'd have to check svn logs to be sure. Either way, does anyone honestly believe that an in-place upgrade will work for versions prior to 0.16 or so in practice?
        Owen O'Malley made changes -
        Field Original Value New Value
        Project Hadoop Common [ 12310240 ] HDFS [ 12310942 ]
        Key HADOOP-5742 HDFS-259
        Component/s dfs [ 12310710 ]
        Todd Lipcon made changes -
        Parent HDFS-1073 [ 12460987 ]
        Issue Type Task [ 3 ] Sub-task [ 7 ]
        Hide
        Todd Lipcon added a comment -

        I looked through some of the old releases, here are the corresponding layout versions:

        version LAYOUT_VERSION
        0.12.0 -3
        0.13.0 -4
        0.14.0 -7
        0.15.0 -10
        0.16.0 -11
        0.17.0 -13
        0.18.0 -16
        0.19.0 -18

        Currently LAST_UPGRADABLE_LAYOUT_VERSION is -7, corresponding to 0.14. LAST_PRE_UPGRADE_LAYOUT_VERSION is -3. So, we can't upgrade from anything below 0.14 regardless. I don't know of anyone running anything that old anymore (it's more than 3 years ago!) and I think we may as well drop these crutches which were added by HADOOP-1242.

        I think we can also get rid fo the isConversionNeeded() functions, etc, which deal with upgrade from this kind of old version.

        Show
        Todd Lipcon added a comment - I looked through some of the old releases, here are the corresponding layout versions: version LAYOUT_VERSION 0.12.0 -3 0.13.0 -4 0.14.0 -7 0.15.0 -10 0.16.0 -11 0.17.0 -13 0.18.0 -16 0.19.0 -18 Currently LAST_UPGRADABLE_LAYOUT_VERSION is -7, corresponding to 0.14. LAST_PRE_UPGRADE_LAYOUT_VERSION is -3. So, we can't upgrade from anything below 0.14 regardless. I don't know of anyone running anything that old anymore (it's more than 3 years ago!) and I think we may as well drop these crutches which were added by HADOOP-1242 . I think we can also get rid fo the isConversionNeeded() functions, etc, which deal with upgrade from this kind of old version.
        Hide
        Todd Lipcon added a comment -

        This patch simply removes the code that creates this directory and looks for it.

        Show
        Todd Lipcon added a comment - This patch simply removes the code that creates this directory and looks for it.
        Todd Lipcon made changes -
        Attachment hdfs-259.txt [ 12457719 ]
        Todd Lipcon made changes -
        Assignee Todd Lipcon [ tlipcon ]
        Todd Lipcon made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Affects Version/s 0.22.0 [ 12314241 ]
        Hide
        Konstantin Shvachko added a comment -

        > we can't upgrade from anything below 0.14 regardless.

        we can, it just takes two steps instead of one. See the Release Note in HADOOP-2797.

        We can safely remove the directory now. It should have been done in HADOOP-2797, but we missed it at the time.

        BTW, I know people who were recently running 0.15. Probably still do.

        Show
        Konstantin Shvachko added a comment - > we can't upgrade from anything below 0.14 regardless. we can, it just takes two steps instead of one. See the Release Note in HADOOP-2797 . We can safely remove the directory now. It should have been done in HADOOP-2797 , but we missed it at the time. BTW, I know people who were recently running 0.15. Probably still do.
        Hide
        Konstantin Shvachko added a comment -

        > This patch simply removes the code that creates this directory and looks for it.

        Not so easy. You should guarantee that it throws an exception, when somebody tries to upgrade from a very old version. The exception should explain what steps are required to get to current version. That is why I think removing isConversionNeeded() is a mistake.
        Let me know if you need old images for testing. I keep those handy just in case.

        Show
        Konstantin Shvachko added a comment - > This patch simply removes the code that creates this directory and looks for it. Not so easy. You should guarantee that it throws an exception, when somebody tries to upgrade from a very old version. The exception should explain what steps are required to get to current version. That is why I think removing isConversionNeeded() is a mistake. Let me know if you need old images for testing. I keep those handy just in case.
        Hide
        Todd Lipcon added a comment -

        Thanks for the feedback, Konst. I will add some code to check for an old version and explain that they must upgrade to 0.20 first as a stepping stone, if you think it's necessary.

        I also realized there are some tests that need a little update. I'll upload a new patch with these changes in the next couple days.

        Show
        Todd Lipcon added a comment - Thanks for the feedback, Konst. I will add some code to check for an old version and explain that they must upgrade to 0.20 first as a stepping stone, if you think it's necessary. I also realized there are some tests that need a little update. I'll upload a new patch with these changes in the next couple days.
        Todd Lipcon made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Todd Lipcon added a comment -

        What do you think about this version, Konstantin? I checked out hadoop 0.3.0, and miraculously it compiled and even formatted a NN image for me New unit test reproduces that image and checks that upgrade continues to fail with the correct message. I took the liberty of renaming some of the functions to be more obvious what they do.

        Show
        Todd Lipcon added a comment - What do you think about this version, Konstantin? I checked out hadoop 0.3.0, and miraculously it compiled and even formatted a NN image for me New unit test reproduces that image and checks that upgrade continues to fail with the correct message. I took the liberty of renaming some of the functions to be more obvious what they do.
        Todd Lipcon made changes -
        Attachment hdfs-259.txt [ 12457800 ]
        Todd Lipcon made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Todd Lipcon added a comment -

        Oops, grabbed the wrong patch file with previous upload. This one has the new test I described.

        Show
        Todd Lipcon added a comment - Oops, grabbed the wrong patch file with previous upload. This one has the new test I described.
        Todd Lipcon made changes -
        Attachment hdfs-259.txt [ 12457801 ]
        Hide
        Todd Lipcon added a comment -

        Ran through tests here. All passed except for known trunk failures.

        Show
        Todd Lipcon added a comment - Ran through tests here. All passed except for known trunk failures.
        Hide
        Eli Collins added a comment -

        +1

        Latest patch looks great. Please update http://wiki.apache.org/hadoop/Hadoop_Upgrade the release notes field on this jira. As you mentioned, it probably doesn't work anyway but good to call out when we explicitly removed this.

        Show
        Eli Collins added a comment - +1 Latest patch looks great. Please update http://wiki.apache.org/hadoop/Hadoop_Upgrade the release notes field on this jira. As you mentioned, it probably doesn't work anyway but good to call out when we explicitly removed this.
        Hide
        Eli Collins added a comment -

        Latest patch plus fixes an existing typo in Storage.java

        Show
        Eli Collins added a comment - Latest patch plus fixes an existing typo in Storage.java
        Eli Collins made changes -
        Attachment hdfs-259.txt [ 12458268 ]
        Hide
        Eli Collins added a comment -

        Latest patch:

             [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 6 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 warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
             [exec] 
             [exec]     +1 system tests framework.  The patch passed system tests framework compile.
             [exec] 
        
        Show
        Eli Collins added a comment - Latest patch: [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 6 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 warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system tests framework. The patch passed system tests framework compile. [exec]
        Hide
        Eli Collins added a comment -

        I've comitted this. Leaving open so we don't forget to update the wiki/release note field.

        Show
        Eli Collins added a comment - I've comitted this. Leaving open so we don't forget to update the wiki/release note field.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #420 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/420/)

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #420 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/420/ )
        Hide
        Konstantin Shvachko added a comment -

        Sorry for late reply. The patch looks good. Thanks Todd.

        Show
        Konstantin Shvachko added a comment - Sorry for late reply. The patch looks good. Thanks Todd.
        Hide
        Konstantin Shvachko added a comment -

        We forgot one important thing here!!!
        The LAYOUT_VERSION should be changed in order to force the upgrade for this change.
        Can we fix it really promptly.
        The LV change should go with the code, so the best way is to revert and re-commit.

        Show
        Konstantin Shvachko added a comment - We forgot one important thing here!!! The LAYOUT_VERSION should be changed in order to force the upgrade for this change. Can we fix it really promptly. The LV change should go with the code, so the best way is to revert and re-commit.
        Konstantin Shvachko made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Fix Version/s 0.22.0 [ 12314241 ]
        Hide
        Eli Collins added a comment -

        Good catch Konstantin, I reverted the change. Todd's out of the office, if he doesn't update the patch soon I'll get to it.

        Show
        Eli Collins added a comment - Good catch Konstantin, I reverted the change. Todd's out of the office, if he doesn't update the patch soon I'll get to it.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #421 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/421/)
        Revert HDFS-259, need to update LAYOUT_VERSION.

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #421 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/421/ ) Revert HDFS-259 , need to update LAYOUT_VERSION.
        Hide
        Todd Lipcon added a comment -

        This version bumps FSImage.LAYOUT_VERSION

        Show
        Todd Lipcon added a comment - This version bumps FSImage.LAYOUT_VERSION
        Todd Lipcon made changes -
        Attachment hdfs-259.txt [ 12459580 ]
        Todd Lipcon made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Todd Lipcon added a comment -

        HDFS-1500 reminded me I had to edit the ImageLoaderCurrent class too.

        Show
        Todd Lipcon added a comment - HDFS-1500 reminded me I had to edit the ImageLoaderCurrent class too.
        Todd Lipcon made changes -
        Attachment hdfs-259.txt [ 12459635 ]
        Hide
        Eli Collins added a comment -

        +1 Latest patch looks good. I ran test-core and verified the test failures were the ones currently failing on trunk.

             [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 6 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 warnings.
             [exec] 
             [exec]     -1 release audit.  The applied patch generated 97 release audit warnings (more than the trunk's current 1 warnings).
             [exec] 
             [exec]     +1 system test framework.  The patch passed system test framework compile.
             [exec] 
        
        Show
        Eli Collins added a comment - +1 Latest patch looks good. I ran test-core and verified the test failures were the ones currently failing on trunk. [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 6 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 warnings. [exec] [exec] -1 release audit. The applied patch generated 97 release audit warnings (more than the trunk's current 1 warnings). [exec] [exec] +1 system test framework. The patch passed system test framework compile. [exec]
        Hide
        Eli Collins added a comment -

        I've committed this. Thanks Todd!

        Show
        Eli Collins added a comment - I've committed this. Thanks Todd!
        Eli Collins made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Konstantin Shvachko made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        8d 58m 2 Konstantin Shvachko 29/Oct/10 21:12
        Open Open Patch Available Patch Available
        561d 7h 29m 3 Todd Lipcon 15/Nov/10 03:37
        Patch Available Patch Available Resolved Resolved
        2d 17h 2m 1 Eli Collins 17/Nov/10 20:39
        Resolved Resolved Closed Closed
        389d 9h 39m 1 Konstantin Shvachko 12/Dec/11 06:19

          People

          • Assignee:
            Todd Lipcon
            Reporter:
            Todd Lipcon
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development