Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1217

Some methods in the NameNdoe should not be public

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.23.2
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      There are quite a few NameNode methods which are not required to be public.

        Activity

        Arun C Murthy made changes -
        Fix Version/s 0.23.2 [ 12319852 ]
        Fix Version/s 0.23.0 [ 12315571 ]
        Fix Version/s 0.24.0 [ 12317653 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #801 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/801/)
        HDFS-1217. Change some NameNode methods from public to package private. Constributed by Laxman

        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #801 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/801/ ) HDFS-1217 . Change some NameNode methods from public to package private. Constributed by Laxman szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #776 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/776/)
        HDFS-1217. Change some NameNode methods from public to package private. Constributed by Laxman

        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #776 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/776/ ) HDFS-1217 . Change some NameNode methods from public to package private. Constributed by Laxman szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #819 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/819/)
        HDFS-1217. Change some NameNode methods from public to package private. Constributed by Laxman

        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #819 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/819/ ) HDFS-1217 . Change some NameNode methods from public to package private. Constributed by Laxman szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #886 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/886/)
        HDFS-1217. Change some NameNode methods from public to package private. Constributed by Laxman

        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #886 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/886/ ) HDFS-1217 . Change some NameNode methods from public to package private. Constributed by Laxman szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #809 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/809/)
        HDFS-1217. Change some NameNode methods from public to package private. Constributed by Laxman

        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #809 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/809/ ) HDFS-1217 . Change some NameNode methods from public to package private. Constributed by Laxman szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1163081 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMultipleRegistrations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
        Tsz Wo Nicholas Sze made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Fix Version/s 0.23.0 [ 12315571 ]
        Fix Version/s 0.24.0 [ 12317653 ]
        Resolution Fixed [ 1 ]
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I have committed this. Thanks, Lakshman!

        Show
        Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Lakshman!
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I also ran the tests. TestHost2NodesMap did not fail and TestOfflineEditsViewer failed with or without the patch.

        Show
        Tsz Wo Nicholas Sze added a comment - I also ran the tests. TestHost2NodesMap did not fail and TestOfflineEditsViewer failed with or without the patch.
        Hide
        Laxman added a comment -

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

        +1 tests included. The patch appears to include 9 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 (version 1.3.9) warnings.

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

        Hi Nicholas,

        Thanks a lot for taking a look.

        When i ran the tests with this patch, TestHost2NodesMap and TestOfflineEditsViewer tests are failing.
        Looks they are not related.

        Show
        Laxman added a comment - +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. Hi Nicholas, Thanks a lot for taking a look. When i ran the tests with this patch, TestHost2NodesMap and TestOfflineEditsViewer tests are failing. Looks they are not related.
        Tsz Wo Nicholas Sze made changes -
        Assignee Laxman [ lakshman ]
        Hadoop Flags [Reviewed]
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Hi Lakshman,

        +1 the patch looks good. Thanks!

        Could you run the unit tests? Hudson did not run any test in the previous build.

        Show
        Tsz Wo Nicholas Sze added a comment - Hi Lakshman, +1 the patch looks good. Thanks! Could you run the unit tests? Hudson did not run any test in the previous build.
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 9 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 (version 1.3.9) warnings.

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

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

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1172//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1172//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1172//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/12491975/HDFS-1217.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1172//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1172//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1172//console This message is automatically generated.
        Laxman made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Laxman added a comment -

        Hi Nicholas,
        I updated the patch for review!

        Thanks,
        Lakshman

        Show
        Laxman added a comment - Hi Nicholas, I updated the patch for review! Thanks, Lakshman
        Laxman made changes -
        Field Original Value New Value
        Attachment HDFS-1217.patch [ 12491975 ]
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Here is some suggestions:

        +++ src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java	(working copy)
        @@ -1137,7 +1137,7 @@
            * @param nodeReg data node registration
            * @throws IOException
            */
        -  public void verifyRequest(NodeRegistration nodeReg) throws IOException {
        +  void verifyRequest(NodeRegistration nodeReg) throws IOException {
             verifyVersion(nodeReg.getVersion());
             if (!namesystem.getRegistrationID().equals(nodeReg.getRegistrationID()))
               throw new UnregisteredNodeException(nodeReg);
        @@ -1149,19 +1149,12 @@
            * @param version
            * @throws IOException
            */
        -  public void verifyVersion(int version) throws IOException {
        +  private static void verifyVersion(int version) throws IOException {
             if (version != LAYOUT_VERSION)
               throw new IncorrectVersionException(version, "data node");
           }
         
        -  /**
        -   * Returns the name of the fsImage file
        -   */
        -  public File getFsImageName() throws IOException {
        -    return getFSImage().getFsImageName();
        -  }
        -    
        -  public FSImage getFSImage() {
        +  FSImage getFSImage() {
             return namesystem.dir.fsImage;
           }
         
        @@ -1169,7 +1162,7 @@
            * Returns the name of the fsImage file uploaded by periodic
            * checkpointing
            */
        -  public File[] getFsImageNameCheckpoint() throws IOException {
        +  File[] getFsImageNameCheckpoint() throws IOException {
             return getFSImage().getFsImageNameCheckpoint();
           }
         
        @@ -1187,7 +1180,7 @@
            * 
            * @return the http address.
            */
        -  public InetSocketAddress getHttpAddress() {
        +  InetSocketAddress getHttpAddress() {
             return httpAddress;
           }
        
        Show
        Tsz Wo Nicholas Sze added a comment - Here is some suggestions: +++ src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java (working copy) @@ -1137,7 +1137,7 @@ * @param nodeReg data node registration * @ throws IOException */ - public void verifyRequest(NodeRegistration nodeReg) throws IOException { + void verifyRequest(NodeRegistration nodeReg) throws IOException { verifyVersion(nodeReg.getVersion()); if (!namesystem.getRegistrationID().equals(nodeReg.getRegistrationID())) throw new UnregisteredNodeException(nodeReg); @@ -1149,19 +1149,12 @@ * @param version * @ throws IOException */ - public void verifyVersion( int version) throws IOException { + private static void verifyVersion( int version) throws IOException { if (version != LAYOUT_VERSION) throw new IncorrectVersionException(version, "data node" ); } - /** - * Returns the name of the fsImage file - */ - public File getFsImageName() throws IOException { - return getFSImage().getFsImageName(); - } - - public FSImage getFSImage() { + FSImage getFSImage() { return namesystem.dir.fsImage; } @@ -1169,7 +1162,7 @@ * Returns the name of the fsImage file uploaded by periodic * checkpointing */ - public File[] getFsImageNameCheckpoint() throws IOException { + File[] getFsImageNameCheckpoint() throws IOException { return getFSImage().getFsImageNameCheckpoint(); } @@ -1187,7 +1180,7 @@ * * @ return the http address. */ - public InetSocketAddress getHttpAddress() { + InetSocketAddress getHttpAddress() { return httpAddress; }
        Tsz Wo Nicholas Sze created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development