Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1080

SecondaryNameNode image transfer should use the defined http address rather than local ip address

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.22.0
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed

      Description

      Currently when telling the NN where to get the merged image, SNN uses getLocalHost.getAddr(), which may not return the correct ip addr.

      1. HDFS-1080-Y20.patch
        0.8 kB
        Jakob Homan
      2. HDFS-1080-trunk.patch
        0.8 kB
        Jakob Homan
      3. HDFS-1080-trunk-1.patch
        3 kB
        Jakob Homan

        Issue Links

          Activity

          Konstantin Shvachko made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #310 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/310/)

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #310 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/310/ )
          Jakob Homan made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags [Incompatible change] [Incompatible change, Reviewed]
          Fix Version/s 0.22.0 [ 12314241 ]
          Resolution Fixed [ 1 ]
          Hide
          Jakob Homan added a comment -

          I've committed this. Resolving as fixed.

          Show
          Jakob Homan added a comment - I've committed this. Resolving as fixed.
          Hide
          Jakob Homan added a comment -

          Failed tests are unrelated. I'm going to go ahead and commit this.

          Show
          Jakob Homan added a comment - Failed tests are unrelated. I'm going to go ahead and commit 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/12446633/HDFS-1080-trunk-1.patch
          against trunk revision 952861.

          +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 new tests are needed for this patch.
          Also please list what manual steps were performed to verify 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 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/Hdfs-Patch-h5.grid.sp2.yahoo.net/402/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/402/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/402/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/402/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/12446633/HDFS-1080-trunk-1.patch against trunk revision 952861. +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 new tests are needed for this patch. Also please list what manual steps were performed to verify 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 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/Hdfs-Patch-h5.grid.sp2.yahoo.net/402/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/402/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/402/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/402/console This message is automatically generated.
          Jakob Homan made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Jakob Homan added a comment -

          Re-submitting to Hudson for new patch.

          Show
          Jakob Homan added a comment - Re-submitting to Hudson for new patch.
          Jakob Homan made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Konstantin Shvachko added a comment -

          +1.
          Yes, this is incompatible in the sense that currently NN and SNN can communicate if SNN uses local host - the default value for its http address, while after the patch they wont be able to. I also think this is a correct behavior. We should clearly mark this in release notes.

          Show
          Konstantin Shvachko added a comment - +1. Yes, this is incompatible in the sense that currently NN and SNN can communicate if SNN uses local host - the default value for its http address, while after the patch they wont be able to. I also think this is a correct behavior. We should clearly mark this in release notes.
          Jakob Homan made changes -
          Hadoop Flags [Incompatible change]
          Jakob Homan made changes -
          Attachment HDFS-1080-trunk-1.patch [ 12446633 ]
          Hide
          Jakob Homan added a comment -

          Updated patch per Konstantin's comments. Checkpointer needed a bit more work to get the relevant information to the right to the needed function, but should work exactly the same. I'm going to open a JIRA to combine this code duplication until the 2ndarryNN is removed.

          Konstantin pointed out that this will be an incompatible change in that, were one specifying localhost as the 2nd/CP Nodes' address and relying on that value to be resolved on the node. For now on, one will need to specify the exact address to use, which is much more correct in my opinion, particularly on production systems that may rely on IP aliasing. I'll mark this as incompatible.

          Show
          Jakob Homan added a comment - Updated patch per Konstantin's comments. Checkpointer needed a bit more work to get the relevant information to the right to the needed function, but should work exactly the same. I'm going to open a JIRA to combine this code duplication until the 2ndarryNN is removed. Konstantin pointed out that this will be an incompatible change in that, were one specifying localhost as the 2nd/CP Nodes' address and relying on that value to be resolved on the node. For now on, one will need to specify the exact address to use, which is much more correct in my opinion, particularly on production systems that may rely on IP aliasing. I'll mark this as incompatible.
          Hide
          Hadoop QA added a comment -

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

          +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 new tests are needed for this patch.
          Also please list what manual steps were performed to verify 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 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/Hdfs-Patch-h5.grid.sp2.yahoo.net/399/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/399/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/399/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/399/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/12446514/HDFS-1080-trunk.patch against trunk revision 951555. +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 new tests are needed for this patch. Also please list what manual steps were performed to verify 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 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/Hdfs-Patch-h5.grid.sp2.yahoo.net/399/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/399/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/399/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/399/console This message is automatically generated.
          Hide
          Konstantin Shvachko added a comment -
          1. You might want also to remove the import of InetAddress, as it is not used anywhere after the change.
          2. The same code is used in Checkpointer.uploadCheckpoint(), which is a part of BackupNode. Should we replace it there as well?
          Show
          Konstantin Shvachko added a comment - You might want also to remove the import of InetAddress , as it is not used anywhere after the change. The same code is used in Checkpointer.uploadCheckpoint() , which is a part of BackupNode . Should we replace it there as well?
          Jakob Homan made changes -
          Link This issue is related to HDFS-1191 [ HDFS-1191 ]
          Jakob Homan made changes -
          Link This issue is related to HDFS-62 [ HDFS-62 ]
          Hide
          gary murry added a comment -

          This should be fine without new unit tests. It has been tested both directly and indirectly.

          Show
          gary murry added a comment - This should be fine without new unit tests. It has been tested both directly and indirectly.
          Jakob Homan made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Jakob Homan added a comment -

          Submitting patch.

          Show
          Jakob Homan added a comment - Submitting patch.
          Jakob Homan made changes -
          Attachment HDFS-1080-trunk.patch [ 12446514 ]
          Hide
          Jakob Homan added a comment -

          Patch for trunk. Exactly the same as the 20 patch, which we've tested manually extensively here. This code is buried quite deep in the guts of the secondary namenode; I think a new unit test isn't feasible here, without a serious, potentially destabilizing refactor. I'd like to go ahead without one.

          This issue occurs, and prevents the 2ndNN from functioning correctly, when a machine is running on a secure cluster and using IP aliasing to run as a different host than is returned from getLocalHost. The NN will attempt to connect to the local host (say 192.168.0.1) rather than the alias (say secure-2nn), and this will fail the Kerberized SSL authentication and prevent the merged image from being downloaded.

          Show
          Jakob Homan added a comment - Patch for trunk. Exactly the same as the 20 patch, which we've tested manually extensively here. This code is buried quite deep in the guts of the secondary namenode; I think a new unit test isn't feasible here, without a serious, potentially destabilizing refactor. I'd like to go ahead without one. This issue occurs, and prevents the 2ndNN from functioning correctly, when a machine is running on a secure cluster and using IP aliasing to run as a different host than is returned from getLocalHost. The NN will attempt to connect to the local host (say 192.168.0.1) rather than the alias (say secure-2nn), and this will fail the Kerberized SSL authentication and prevent the merged image from being downloaded.
          Jakob Homan made changes -
          Field Original Value New Value
          Attachment HDFS-1080-Y20.patch [ 12440810 ]
          Hide
          Jakob Homan added a comment -

          Patch to use defined addr, for Y!20. Trunk patch soon...

          Show
          Jakob Homan added a comment - Patch to use defined addr, for Y!20. Trunk patch soon...
          Jakob Homan created issue -

            People

            • Assignee:
              Jakob Homan
              Reporter:
              Jakob Homan
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development