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

Zhijie Shen commented on YARN-3116:
-----------------------------------

Looks good to me overall. Two nits:

1. I thought it may not be necessary, as they should be not null at the moment 
to allocate a container.
{code}
479           if (appAttempt != null && appAttempt.getSubmissionContext() != 
null) {
{code}

2. Can we avoid duplicating most of the code in 
"testAMContainerTokenIdentifier"?

> [Collector wireup] We need an assured way to determine if a container is an 
> AM container on NM
> ----------------------------------------------------------------------------------------------
>
>                 Key: YARN-3116
>                 URL: https://issues.apache.org/jira/browse/YARN-3116
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager, timelineserver
>            Reporter: Zhijie Shen
>            Assignee: Giovanni Matteo Fumarola
>         Attachments: YARN-3116.patch, YARN-3116.v2.patch, YARN-3116.v3.patch
>
>
> In YARN-3030, to start the per-app aggregator only for a started AM 
> container,  we need to determine if the container is an AM container or not 
> from the context in NM (we can do it on RM). This information is missing, 
> such that we worked around to considered the container with ID "xxxx_01" as 
> the AM container. Unfortunately, this is neither necessary or sufficient 
> condition. We need to have a way to determine if a container is an AM 
> container on NM. We can add flag to the container object or create an API to 
> do the judgement. Perhaps the distributed AM information may also be useful 
> to YARN-2877.



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

Reply via email to