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

Zhijie Shen commented on YARN-3334:
-----------------------------------

Junping, thanks for the patch. Here's my comments:

1. Do you want change "initialized" to "started" 
{code}
512             // only put initialized client
{code}

2. The following method seems unnecessary, because there's 
{{getTimelineClient(ApplicationId id)}}.
{code}
499         
500         public Map<ApplicationId, TimelineClient> getTimelineClients() {
501           return this.timelineClients;
502         }
503     
{code}

3. It seems there's no need to maintain rmKnownCollectors. We can blindly put 
the service addr into timeline client. It won't affect anything if the address 
is not changed? Or we can do a simple check {{client.getAddr != 
newServiceAddr}} to avoid trivial set.

4. IMHO, the better description is to use ContainerEntity whose ID is this 
container ID.
{code}
441               TimelineEntity entity = new TimelineEntity();
442               entity.setType(NMEntity.NM_CONTAINER_METRICS.toString());
443               entity.setId(containerId.toString());
{code}

5. We need flag to control NM emitting the timeline data or not.

6. Unnecessary empty string.
{code}
        503                     "" + cpuUsageTotalCoresPercentage);
{code}

7. You probably want to use addTimeSeriesData to add single key/value pair.
{code}
526                 memoryMetric.setTimeSeries(timeSeries);
{code}

8. NM needs to remove the timelineClient of a finished app. Otherwise, 
timelineClients will eat increasingly more resource as NM keeps running, but 
actually don't use it. The difficulty is how to know if an application is 
already finished. We need to think about it.
{code}
368         private ConcurrentHashMap<ApplicationId, TimelineClient> 
timelineClients = 
369             new ConcurrentHashMap<ApplicationId, TimelineClient>();
{code}

> [Event Producers] NM start to posting some app related metrics in early POC 
> stage of phase 2.
> ---------------------------------------------------------------------------------------------
>
>                 Key: YARN-3334
>                 URL: https://issues.apache.org/jira/browse/YARN-3334
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager
>    Affects Versions: YARN-2928
>            Reporter: Junping Du
>            Assignee: Junping Du
>         Attachments: YARN-3334-demo.patch, YARN-3334-v1.patch
>
>




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

Reply via email to