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

Bikas Saha commented on YARN-752:
---------------------------------

First of all I think I made a mistake in putting ImmutableList as part of the 
signature of the getHosts() etc. It ties down the implementation vs having 
returned a generic list. We should probably fix that. I can do that in a 
different jira. We should be storing a simple memory efficient list and 
returning an unmodifiablelist instead of ImmutableList.

Changing the storage type to set is not desired. The set is only needed 1 time 
for ease of lookup(and probably isnt much different in perf vs linear search 
for single digit sizes). Lets not change the actual storage to a set because 
its just more memory (for millions of container request) and it also loses the 
ordering given by the user. At some point we may want to match requests in the 
preferred order given by the user.

What if the RackResolver is improperly initialized because of a bad config? And 
does not return valid/non-null values? Can you please check?
{code}
+        String rack = RackResolver.resolve(host).getNetworkLocation();
+        if (!req.racks.contains(rack)) {
{code}

The test could probably go into the existing tests for AMRMClient so that it 
stays in one place. Also, would be great to make sure that 
getMatchingRequests() work properly in this case - user should get the 
container request for a rack for which they had originally specified only 
nodes. They should but a test would verify the expected behavior.
                
> In AMRMClient, automatically add corresponding rack requests for requested 
> nodes
> --------------------------------------------------------------------------------
>
>                 Key: YARN-752
>                 URL: https://issues.apache.org/jira/browse/YARN-752
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: api, applications
>    Affects Versions: 2.0.4-alpha
>            Reporter: Sandy Ryza
>            Assignee: Sandy Ryza
>         Attachments: YARN-752.patch
>
>
> A ContainerRequest that includes node-level requests must also include 
> matching rack-level requests for the racks that those nodes are on.  When a 
> node is present without its rack, it makes sense for the client to 
> automatically add the node's rack.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to