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

Daniel Templeton commented on HDFS-9112:
----------------------------------------

Thanks, Anu, I haven't tested the patch yet, but from reading the changes, here 
are my initial comments.  (I'm playing with formatting options for the code 
macro.  Hopefully this comes through as intended.)

{code:title=DFSUtil.java|linenumbers=true|firstline=1148}
    // HDFS-6376 Added the ability to support multiple NameService IDs.
    // if we have multiple nameService IDs we need to read
    // dfs.internal.nameservices and pick that name as the internal Name
    // Service Name.
{code}

Love that you're explaining what you're doing, but let's leave the JIRA ID out 
of it and don't explain it in terms of what changed.  The comment should just 
explain the current state of things.

{code:title=DFSUtil.java|linenumbers=true|firstline=1169}
    return internalNSIds.toArray(new String[1])[0];
{code}

I think {code}internalNSIds.iterator().next(){code} is the more standard way to 
do that.

{code:title=HdfsClientConfigKeys.java|linenumbers=true|firstline=49}
  String DFS_INTERNAL_NAMESERVICES_KEY = "dfs.internal.nameservices";
{code}

I realize that you're just following the pattern set with DFS_NAMESERVICES_KEY, 
but I really don't like the idea of duplicating key names between DFSConfigKeys 
and HdfsClientConfigKeys.  If there some reason not to use 
DFSConfigKeys.DFS_INTERNAL_NAMESERVICES_KEY?



> Haadmin fails if multiple name service IDs are configured
> ---------------------------------------------------------
>
>                 Key: HDFS-9112
>                 URL: https://issues.apache.org/jira/browse/HDFS-9112
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: tools
>    Affects Versions: 2.7.1
>            Reporter: Anu Engineer
>            Assignee: Anu Engineer
>         Attachments: HDFS-9112.001.patch, HDFS-9112.002.patch
>
>
> In HDFS-6376 we supported a feature for distcp that allows multiple 
> NameService IDs to be specified so that we can copy from two HA enabled 
> clusters.
> That confuses haadmin command since we have a check in 
> DFSUtil#getNamenodeServiceAddr which fails if it finds more than 1 name in 
> that property.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to