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

Janus Chow commented on HDFS-15579:
-----------------------------------

There is already some instruction for this constructor.
{code:java}
/**
 * Create a path location from another path with the destinations sorted.
 *
 * @param other Other path location to copy from.
 * @param firstNsId Identifier of the namespace to place first.
 */
{code}
The problem comes to me is that when I was reading the code of 
MultipleDestinationMountTableResolver calling this constructor, the first idea 
comes to me is that this constructor is to create a PathLocation with a 
specified nsId, and I read on, I wouldn't get into the class to check more 
details about this constructor.

So I wonder if it can be more specific from the name, like a function named 
"sortDestination" returning a new PathLocation instead of a constructor. Just 
wondering.

> RBF: The constructor of PathLocation may got some misunderstanding
> ------------------------------------------------------------------
>
>                 Key: HDFS-15579
>                 URL: https://issues.apache.org/jira/browse/HDFS-15579
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: rbf
>            Reporter: Janus Chow
>            Priority: Minor
>
> There is a constructor of PathLocation as follows, it's for creating a new 
> PathLocation with a prioritised nsId. 
>  
> {code:java}
> public PathLocation(PathLocation other, String firstNsId) {
>   this.sourcePath = other.sourcePath;
>   this.destOrder = other.destOrder;
>   this.destinations = orderedNamespaces(other.destinations, firstNsId);
> }
> {code}
> When I was reading the code of MultipleDestinationMountTableResolver, I 
> thought this constructor was to create a PathLocation with an override 
> destination. It took me a while before I realize this is a constructor to 
> sort the destinations inside.
> Maybe I think this constructor can be more clear about its usage?
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to