[jira] Commented: (HADOOP-7030) new topology mapping implementations

["Patrick Angeles (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-7030?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13002039#comment-13002039 ] 

Patrick Angeles commented on HADOOP-7030:
-----------------------------------------

Hey Tom,

Thanks for the review. Here are some responses:

Could you combine the two types of file, so that if there are three columns the first two are interpreted as a range, otherwise use the first as a single host. Or just support CIDR notation?

- I'd prefer to keep them separate as the first two columns have completely different meanings when using one style (table lookup) over the other (IP-range).

Have you thought about InetAddress to avoid implementing IP address parsing logic? http://guava-libraries.googlecode.com/svn/tags/release08/javadoc/com/google/common/net/InetAddresses.html might be useful (there was talk of introducing Guava recently).

- I have not. Will look into this, although I'd rather keep this patch lightweight and not require the addition of another jar.

RefreshableDNSToSwitchMapping isn't hooked up yet, so perhaps it should go in a follow on JIRA.

- Yes, that is the intent. Those JIRAs would go into MAPREDUCE and HDFS.

The name "TableMapping" is a bit general. How about "FileBasedMapping", or similar?

- I'm willing to listen to suggestions, however I think FileBasedMapping is even more vague :)

The configuration keys should go in CommonConfigurationKeysPublic.

- Since this is a pluggable interface, I should not have to modify existing core code. That's better WRT separation of concerns and componentization. I'm willing to take your suggestion if the general consensus is that I should :)

Primes are not needed in hashCode implementations. For Ip4 Arrays.hashCode(value) is sufficient.

- Ok.

The tests swallow exceptions - there should at least be a comment saying that this is expected. Also, fail() with a message is preferable to assertTrue(false).

- Ok.

The tests should be JUnit 4 style.

- Ok.

> new topology mapping implementations
> ------------------------------------
>
>                 Key: HADOOP-7030
>                 URL: https://issues.apache.org/jira/browse/HADOOP-7030
>             Project: Hadoop Common
>          Issue Type: New Feature
>    Affects Versions: 0.20.1, 0.20.2, 0.21.0
>            Reporter: Patrick Angeles
>            Assignee: Patrick Angeles
>         Attachments: HADOOP-7030-2.patch, HADOOP-7030.patch, topology.patch
>
>
> The default ScriptBasedMapping implementation of DNSToSwitchMapping for determining cluster topology has some drawbacks. Principally, it forks to an OS-specific script.
> This issue proposes two new Java implementations of DNSToSwitchMapping. TableMapping reads a two column text file that maps an IP or hostname to a rack ID. Ip4RangeMapping reads a three column text file where each line represents a start and end IP range plus a rack ID.

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira