Hadoop Common
  1. Hadoop Common
  2. HADOOP-2413 Is FSNamesystem.fsNamesystemObject unique?
  3. HADOOP-5120

UpgradeManagerNamenode and UpgradeObjectNamenode should not use FSNamesystem.getFSNamesystem()

    Details

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

      Description

      UpgradeManagerNamenode and UpgradeObjectNamenode should not access the namespace by the static method FSNamesystem.getFSNamesystem()

      1. 5120_20090202.patch
        5 kB
        Tsz Wo Nicholas Sze

        Activity

        Hide
        Tsz Wo Nicholas Sze added a comment -

        5120_20090202.patch: removed the uses of FSNamesystem.getFSNamesystem()

        Show
        Tsz Wo Nicholas Sze added a comment - 5120_20090202.patch: removed the uses of FSNamesystem.getFSNamesystem()
        Hide
        Raghu Angadi added a comment -

        +1.

        This removes a main()... which is unrelated. To keep it or not is up to you. Konstantin might have used it sometime back.

        Show
        Raghu Angadi added a comment - +1. This removes a main()... which is unrelated. To keep it or not is up to you. Konstantin might have used it sometime back.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Tested locally, submitting ...

        Show
        Tsz Wo Nicholas Sze added a comment - Tested locally, submitting ...
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12399309/5120_20090202.patch
        against trunk revision 746010.

        +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 passed core unit tests.

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3886/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3886/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3886/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3886/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/12399309/5120_20090202.patch against trunk revision 746010. +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 passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3886/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3886/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3886/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3886/console This message is automatically generated.
        Hide
        Konstantin Shvachko added a comment -

        +1.
        A wish for future changes in this direction. In many cases it is not necessary to pass the whole FSNamesystem class reference. If only FSImage of FSDirectory is necessary please make constructors take exactly that. Like here, you could have moved a call to FSNamesystem.leaveSafeMode() up to FSNamesystem.processDistributedUpgradeCommand, and then UpgradeManagerNamenode would need only FSImage.
        I am trying to prevent mistakes which break the synchronization order. They are very easy make if you have a direct reference to FSNamesystem.

        Show
        Konstantin Shvachko added a comment - +1. A wish for future changes in this direction. In many cases it is not necessary to pass the whole FSNamesystem class reference. If only FSImage of FSDirectory is necessary please make constructors take exactly that. Like here, you could have moved a call to FSNamesystem.leaveSafeMode() up to FSNamesystem.processDistributedUpgradeCommand, and then UpgradeManagerNamenode would need only FSImage. I am trying to prevent mistakes which break the synchronization order. They are very easy make if you have a direct reference to FSNamesystem.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        The failed test is not related. I committed this.

        Show
        Tsz Wo Nicholas Sze added a comment - The failed test is not related. I committed this.
        Hide
        Hudson added a comment -
        Show
        Hudson added a comment - Integrated in Hadoop-trunk #763 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/763/ )

          People

          • Assignee:
            Tsz Wo Nicholas Sze
            Reporter:
            Tsz Wo Nicholas Sze
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development