[jira] [Commented] (HDFS-3216) DatanodeID should support multiple IP addresses

2012-04-09 Thread Todd Lipcon (Commented) (JIRA)

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

Todd Lipcon commented on HDFS-3216:
---

bq. #2 Yes, when reading/writing DatanodeInfos to/from streams (same as before 
when creating a DatanodeID w/o a name)

When do we read/write DatanodeInfo from streams, now that we are pb-ified? i.e 
is the writable interface even used anymore?


{code}
+   * Return the canonical IP address for this DatanodeID. Not all uses
+   * of DatanodeID are multi-IP aware, or would multiple IPs, therefore
+   * we use the first address as the canonical one.
{code}
ENOTASENTENCE


bq. #1 We still need the notion of canonical IP, mostly for cases that don't 
care about multiple IP addresses. Updated the javadoc.

How is it ensured that the "canonical" IP is kept consistent across DN 
restarts, for example? It's just whichever one is listed first in the DN-side 
configuration?

bq. Fixed the cast, now casts to String and serializes/deserializes the IPs, 
the test does check this (was failing now passes).

That's a little strange, to serialize it into a comma-separated list inside 
JSON. It's not possible to get Jackson to serialize it as a proper JSON array? 
Perhaps using a List inside the map?

> DatanodeID should support multiple IP addresses
> ---
>
> Key: HDFS-3216
> URL: https://issues.apache.org/jira/browse/HDFS-3216
> Project: Hadoop HDFS
>  Issue Type: Sub-task
>Reporter: Eli Collins
>Assignee: Eli Collins
> Attachments: hdfs-3216.txt, hdfs-3216.txt
>
>
> The DatanodeID has a single field for the IP address, for HDFS-3146 we need 
> to extend it to support multiple addresses.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira




[jira] [Commented] (HDFS-3216) DatanodeID should support multiple IP addresses

2012-04-08 Thread Todd Lipcon (Commented) (JIRA)

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

Todd Lipcon commented on HDFS-3216:
---

Should we deprecate this function? Or do we need some concept of the 
"canonical"/"main" IP address? If the latter, we should explain this in the 
javadoc of this function.

{code}
   public String getIpAddr() {
   -return ipAddr;
   +return ipAddrs[0];
   +  }
{code}



- is it ever valid to construct a DatanodeID with no IP addresses? If not we 
should add a Preconditions check or at least an assert on the length of the 
ipAddrs array in the constructor and the setter



{code}
+return new DatanodeID(ipAddrs.toArray(new String[ipAddrs.size()]) , 
dn.getHostName(), dn.getStorageID(),
 dn.getXferPort(), dn.getInfoPort(), dn.getIpcPort());
{code}
Can you re-wrap this to 80chars?



- Is the code change in JsonUtil covered by TestJsonUtil? (are you sure that 
the cast to String[] is right?)

- in some of the tests, it's filling in hostnames instead of IPs for the 
ipAddrs field. Is that right, or do we expect that it will always be resolved 
IPs? The dual nature makes me nervous.


> DatanodeID should support multiple IP addresses
> ---
>
> Key: HDFS-3216
> URL: https://issues.apache.org/jira/browse/HDFS-3216
> Project: Hadoop HDFS
>  Issue Type: Sub-task
>Reporter: Eli Collins
>Assignee: Eli Collins
> Attachments: hdfs-3216.txt
>
>
> The DatanodeID has a single field for the IP address, for HDFS-3146 we need 
> to extend it to support multiple addresses.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira