[
https://issues.apache.org/jira/browse/YARN-5124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15325276#comment-15325276
]
Karthik Kambatla commented on YARN-5124:
----------------------------------------
Comments on the latest patch:
# AMRMClient: Javdoc for the new method refers to the other method as "above".
It is okay to duplicate the comment to the extent needed.
# AMRMClientAsync: Instead of throwing unsupported, shouldn't the method be
calling client.getMatchingRequests? If throwing an exception is appropriate,
let us update the exception message? It is likely okay to skip the
implementation of this method in AMRMClientAsyncImpl?
# AMRMClientImpl: Unused import Iterator
# RemoteRequestsTable
## RequestInfoIterator does not implement remove.
## The following two lines could be on the same line.
{code}
class RemoteRequestsTable<T> implements
Iterable<ResourceRequestInfo<T>>{
{code}
## When declaring remoteRequestsTable, can we add info on what each level of
the map is for? e.g. It is not immediately obvious what the String in the map
stands for.
## The naming of helper methods getLocationMap etc. can be more explicit. e.g.
getRemoteRequestsForLocation.
## addResourceRequest: Mind filing a follow up JIRA under YARN-1011 and post
this snippet as a JIRA comment so we can get rid of it from the source.
# AMRMClientImpl
## Same as what Carlo mentioned. While iterating over remoteRequestsTable,
change Object to ResourceRequestInfo?
## Nit; In the following snippet, would prefer req and getExecutionTypeRequest
to be on the same line for readability.
{code}
addResourceRequest(req.getPriority(), node, req
.getExecutionTypeRequest(), req.getCapability(), req, true,
req.getNodeLabelExpression());
{code}
> Modify AMRMClient to set the ExecutionType in the ResourceRequest
> -----------------------------------------------------------------
>
> Key: YARN-5124
> URL: https://issues.apache.org/jira/browse/YARN-5124
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Arun Suresh
> Assignee: Arun Suresh
> Attachments: YARN-5124.001.patch, YARN-5124.002.patch,
> YARN-5124.003.patch, YARN-5124.004.patch, YARN-5124.005.patch,
> YARN-5124.006.patch, YARN-5124.008.patch, YARN-5124.009.patch,
> YARN-5124.010.patch, YARN-5124.011.patch, YARN-5124.012.patch,
> YARN-5124.013.patch, YARN-5124.014.patch,
> YARN-5124_YARN-5180_combined.007.patch, YARN-5124_YARN-5180_combined.008.patch
>
>
> Currently the {{ContainerRequest}} allows the AM to set the {{ExecutionType}}
> in the AMRMClient, but it is not being set in the actual {{ResourceRequest}}
> that is sent to the RM
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
