Hadoop Common
  1. Hadoop Common
  2. HADOOP-2413

Is FSNamesystem.fsNamesystemObject unique?

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.15.1
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      FSNamesystem is unique in almost all cases but it is not universally true. So we should either remove the static variable FSNamesystem.fsNamesystemObject or make it final (so that it cannot be overwritten).

      When I am working on HADOOP-1298, I use the convenient static method FSNamesystem.getFSNamesystem() to get "the" FSNamesystem object. However, it keeps failing on TestCheckpoint. Why? It is because TestCheckpoint uses NameNode and SecondaryNameNode. Both of them are creating FSNamesystem. So FSNamesystem.fsNamesystemObject does not remain constant. The kind of bug is hard to be detected.

      1. unStatic.patch
        25 kB
        Konstantin Shvachko

        Issue Links

        There are no Sub-Tasks for this issue.

          Activity

          Hide
          Hudson added a comment -
          Show
          Hudson added a comment - Integrated in Hadoop-trunk #796 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/796/ )
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I've committed this. Thanks, Konstantin!

          Show
          Tsz Wo Nicholas Sze added a comment - I've committed this. Thanks, Konstantin!
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good. Konstantin, thank you for working on this!

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good. Konstantin, thank you for working on this!
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 6 new or modified tests.

          +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-minerva.apache.org/89/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/89/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/89/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/89/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/12404282/unStatic.patch against trunk revision 760783. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +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-minerva.apache.org/89/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/89/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/89/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/89/console This message is automatically generated.
          Hide
          Konstantin Shvachko added a comment -

          Here is a patch, which finally removes the static FSNamesystem.fsNamesystemObject field and the respective method to access it.
          The trick was to eliminate static methods like FSEditsLog.loadFSEdits().
          I also had to change TestEditLog, which was starting MiniDFSCluster only to obtain name directories and then worked directly with FSImage. Now it uses FSImage through teh cluster variable.
          I removed FSDirectory.namesystem member because namesystem is accessible via FSImage now.

          Show
          Konstantin Shvachko added a comment - Here is a patch, which finally removes the static FSNamesystem.fsNamesystemObject field and the respective method to access it. The trick was to eliminate static methods like FSEditsLog.loadFSEdits() . I also had to change TestEditLog , which was starting MiniDFSCluster only to obtain name directories and then worked directly with FSImage . Now it uses FSImage through teh cluster variable. I removed FSDirectory.namesystem member because namesystem is accessible via FSImage now.
          Hide
          Konstantin Shvachko added a comment -

          You probably want to deprecate the static fsNamesystemObject member somewhere along the lines.

          Show
          Konstantin Shvachko added a comment - You probably want to deprecate the static fsNamesystemObject member somewhere along the lines.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          The use of this static varible FSNamesystem.fsNamesystemObject induces the following Findbugs warning:

          • FSNamesystem:365
            [ST] Write to static field from instance method [ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD]
            This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.
          Show
          Tsz Wo Nicholas Sze added a comment - The use of this static varible FSNamesystem.fsNamesystemObject induces the following Findbugs warning: FSNamesystem:365 [ST] Write to static field from instance method [ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD] This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development