Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-alpha
    • Component/s: io, performance
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Common side of HDFS-3148, add necessary DNS and NetUtils methods. Test coverage is in the HDFS-3148 patch.

      1. hadoop-8210.txt
        9 kB
        Eli Collins
      2. hadoop-8210.txt
        10 kB
        Eli Collins

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1039 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1039/)
          HADOOP-8210. Common side of HDFS-3148: The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457)

          Result = FAILURE
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1039 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1039/ ) HADOOP-8210 . Common side of HDFS-3148 : The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457) Result = FAILURE eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1004 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1004/)
          HADOOP-8210. Common side of HDFS-3148: The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457)

          Result = FAILURE
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1004 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1004/ ) HADOOP-8210 . Common side of HDFS-3148 : The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457) Result = FAILURE eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #1982 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1982/)
          HADOOP-8210. Common side of HDFS-3148: The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457)

          Result = FAILURE
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1982 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1982/ ) HADOOP-8210 . Common side of HDFS-3148 : The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457) Result = FAILURE eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Hide
          Eli Collins added a comment -

          Thank you for the reviews, Todd and Daryn! I've committed this and merged to branch-2.

          Show
          Eli Collins added a comment - Thank you for the reviews, Todd and Daryn! I've committed this and merged to branch-2.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #1970 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1970/)
          HADOOP-8210. Common side of HDFS-3148: The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1970 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1970/ ) HADOOP-8210 . Common side of HDFS-3148 : The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #2045 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2045/)
          HADOOP-8210. Common side of HDFS-3148: The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2045 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2045/ ) HADOOP-8210 . Common side of HDFS-3148 : The client should be able to use multiple local interfaces for data transfer. Contributed by Eli Collins (Revision 1308457) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308457 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
          Hide
          Daryn Sharp added a comment -

          +1 this part doesn't look like it will impact tokens

          Show
          Daryn Sharp added a comment - +1 this part doesn't look like it will impact tokens
          Hide
          Todd Lipcon added a comment -

          +1

          Show
          Todd Lipcon added a comment - +1
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12520846/hadoop-8210.txt
          against trunk revision .

          +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 eclipse:eclipse. The patch built with eclipse:eclipse.

          +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:
          org.apache.hadoop.fs.viewfs.TestViewFsTrash

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/809//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/809//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/12520846/hadoop-8210.txt against trunk revision . +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 eclipse:eclipse. The patch built with eclipse:eclipse. +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: org.apache.hadoop.fs.viewfs.TestViewFsTrash +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/809//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/809//console This message is automatically generated.
          Hide
          Eli Collins added a comment -

          Thanks Todd. Updated patch attached.

          Show
          Eli Collins added a comment - Thanks Todd. Updated patch attached.
          Hide
          Todd Lipcon added a comment -
          +    LinkedHashSet<InetAddress> addrs = new LinkedHashSet<InetAddress>();
          

          I think it's worth changing the return type of this function to LinkedHashSet, so it's clear that the ordering here is on purpose. Perhaps also add a comment here saying something like:

          // See below for reasoning behind using an ordered set.
          

          +    // that depend on a particular element being 1st in the array.
          +    // Eg. getDefaultIP always returns the 1st element.
          

          Nits: please un-abbreviate "first" for better readability. Also, "e.g." instead of "Eg." – or just say "For example"


          +      ips[i] = addr.getHostAddress();
          +      i++;
          

          I think it's more idiomatic to just put the postincrement inside the []s


          • there's a small spurious whitespace change in NetUtils.java
          • looks like the pom change is still in this patch (redundant with HADOOP-8211)
          Show
          Todd Lipcon added a comment - + LinkedHashSet<InetAddress> addrs = new LinkedHashSet<InetAddress>(); I think it's worth changing the return type of this function to LinkedHashSet, so it's clear that the ordering here is on purpose. Perhaps also add a comment here saying something like: // See below for reasoning behind using an ordered set. + // that depend on a particular element being 1st in the array. + // Eg. getDefaultIP always returns the 1st element. Nits: please un-abbreviate "first" for better readability. Also, "e.g." instead of "Eg." – or just say "For example" + ips[i] = addr.getHostAddress(); + i++; I think it's more idiomatic to just put the postincrement inside the []s there's a small spurious whitespace change in NetUtils.java looks like the pom change is still in this patch (redundant with HADOOP-8211 )
          Hide
          Daryn Sharp added a comment -

          Very nice doc. I need to study it a bit more and see if/how it meshes with a few ideas I've been pondering with regards to tokens and multiple hostnames and ips. I started thinking of alternatives after seeing the token splitting that HA is currently doing.

          Show
          Daryn Sharp added a comment - Very nice doc. I need to study it a bit more and see if/how it meshes with a few ideas I've been pondering with regards to tokens and multiple hostnames and ips. I started thinking of alternatives after seeing the token splitting that HA is currently doing.
          Hide
          Eli Collins added a comment -

          HDFS-3148, which this jira tracks the common side of, just pertains to outgoing client interfaces, so doesn't pertain to datanode IPs. However, I think your comment is relevant to HADOOP-8198. I think HADOOP-7510 is complimentary, if we don't use hostnames then we'd have to have a separate service token for each interface on the datanode. Could you take a look at the design doc and see if it jives with what you're thinking?

          Show
          Eli Collins added a comment - HDFS-3148 , which this jira tracks the common side of, just pertains to outgoing client interfaces, so doesn't pertain to datanode IPs. However, I think your comment is relevant to HADOOP-8198 . I think HADOOP-7510 is complimentary, if we don't use hostnames then we'd have to have a separate service token for each interface on the datanode. Could you take a look at the design doc and see if it jives with what you're thinking?
          Hide
          Daryn Sharp added a comment -

          I may not be properly understanding the intent of the change, hence my request for clarification. I haven't fully researched the related jiras. I'll quickly explain host based tokens so you can easily respond.

          As you probably already know, the default token implementation always resolves the host to an ip for the token's service. This is fine if the user explicitly specified an ip (maybe due to dns errors, or no dns entries), but the host based tokens preserve the exact hostname or ip the user specified. Preserving the hostname shields the user against ip changes which is very important, for example, when a local or remote NN's ip may change during an upgrade.

          That said, will this change be used exclusively by the client to select an outgoing network interface? That will be fine. My concern is if the client starts always using ips for a remote host since the token will probably have to contain the ip – depending how/where these new methods are used. Resolving an ip back to a hostname is insufficient since the user may have provided a CNAME and a reverse lookup will return the A name. If the CNAME's ip changes, the client will not detect it which defeats the host based tokens.

          Thanks in advance for taking the time to explain.

          Show
          Daryn Sharp added a comment - I may not be properly understanding the intent of the change, hence my request for clarification. I haven't fully researched the related jiras. I'll quickly explain host based tokens so you can easily respond. As you probably already know, the default token implementation always resolves the host to an ip for the token's service. This is fine if the user explicitly specified an ip (maybe due to dns errors, or no dns entries), but the host based tokens preserve the exact hostname or ip the user specified. Preserving the hostname shields the user against ip changes which is very important, for example, when a local or remote NN's ip may change during an upgrade. That said, will this change be used exclusively by the client to select an outgoing network interface? That will be fine. My concern is if the client starts always using ips for a remote host since the token will probably have to contain the ip – depending how/where these new methods are used. Resolving an ip back to a hostname is insufficient since the user may have provided a CNAME and a reverse lookup will return the A name. If the CNAME's ip changes, the client will not detect it which defeats the host based tokens. Thanks in advance for taking the time to explain.
          Hide
          Eli Collins added a comment -

          Why would the local client interface used impact security?

          Show
          Eli Collins added a comment - Why would the local client interface used impact security?
          Hide
          Daryn Sharp added a comment -

          Given the extensive use of IPs in this patch, is this going to impact the ability to use hostnames as a token's service?

          Show
          Daryn Sharp added a comment - Given the extensive use of IPs in this patch, is this going to impact the ability to use hostnames as a token's service?
          Hide
          Eli Collins added a comment -

          Think the change to bump commons-net in hadoop-project/pom.xml needs its own change.

          Show
          Eli Collins added a comment - Think the change to bump commons-net in hadoop-project/pom.xml needs its own change.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12519884/hadoop-8210.txt
          against trunk revision .

          +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 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/779//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/12519884/hadoop-8210.txt against trunk revision . +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 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/779//console This message is automatically generated.
          Hide
          Eli Collins added a comment -

          Patch attached (for trunk).

          Show
          Eli Collins added a comment - Patch attached (for trunk).

            People

            • Assignee:
              Eli Collins
              Reporter:
              Eli Collins
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development