[
https://issues.apache.org/jira/browse/YARN-521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13705547#comment-13705547
]
Bikas Saha commented on YARN-521:
---------------------------------
Sandy, let me start by saying that I really appreciate the effort you are
putting into this. However, I will be cautious about making changes to the API
because once we release it in the beta then it will be hard to change them. If
needed, we can request the release manager to hold the release until we get
this jira done. Let us ensure that the users who want this get a clean and good
experience. To be clear, I am not suggesting that the patch as proposed is far
from it. We are close.
e.g. When you stated above that a node and rack specified together is legal and
gives a preference for the node, its only subtly different from the default
scheduler behavior in the absence of any constraints. Luckily this case is
already covered by the existing code even though we had not explicitly
articulated it in our thought process. Also, 2 requests with specific node and
rack each are different from a single specific request with both. This is
reasonably hard to understand. How can we help the user here?
Let us explicitly point-wise enumerate the cases that are legal and safe. I can
help you with that. This will help me or someone else understand the code and
review it better. If the javadoc is also written in the same manner then it
will help users clearly understand how to use the API. It will also help ensure
that we are testing for those cases.
bq. The way I see it, getMatchingRequests(priority1, * ) should give me all the
requests at priority1, independent of locality relaxation
Thats not quite correct per my intention when I wrote that method. The method
is supposed to give me requests that I can assign based on the parameters of
priority, capability and location. Thats why it checks that resource requests
can actually fit within the capability parameter (thus taking care of scheduler
normalizations). Intention is to have the AMRMClient do the reverse translation
of priority, location, capability because it does the forward translation when
the request is made and therefore is uniquely positioned to make that
determination correctly. Without this, users might have to reverse engineer the
mapping logic for themselves. Hence, its sub-optimal to leave the users to fend
for themselves wrt locality relaxation, specially when we can see that it can
be tricky :) All this is only in the context of StoredContainerRequest's which
constrains the request container count to be 1 (without which the book-keeping
may not be possible in some cases). Solution is simply to store the request
only for the locations on which we can assign it. Thus when a specific node
request comes up, we will store that request in the ResourceRequestInfo of the
node, but not in the ResourceRequestInfo for the inferred rack and ANY. Let me
know if this doesnt make sense or will not be correct.
bq. My intended behavior is that to do this the user must remove the original
container request before adding a new one with different locality relaxation.
Am I misunderstanding how removeContainerRequest works again?
That makes sense. We need to document this. A test would be even better. The
downside to this approach would be that if the async heartbeat to the RM
happens in between these 2 calls then the for some time the RM will think that
the app doesnt need those resources and thus the app can miss some matching
scheduling opportunities. But I think we can live with that.
Looks like we have forgotten StoredContainerRequest in these changes. Its
constructor also needs to account for the new flag.
> Augment AM - RM client module to be able to request containers only at
> specific locations
> -----------------------------------------------------------------------------------------
>
> Key: YARN-521
> URL: https://issues.apache.org/jira/browse/YARN-521
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: api
> Affects Versions: 2.0.3-alpha
> Reporter: Sandy Ryza
> Assignee: Sandy Ryza
> Attachments: YARN-521-1.patch, YARN-521-2.patch, YARN-521-2.patch,
> YARN-521-3.patch, YARN-521.patch
>
>
> When YARN-392 and YARN-398 are completed, it would be good for AMRMClient to
> offer an easy way to access their functionality
--
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
