[
https://issues.apache.org/jira/browse/YARN-3334?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14387463#comment-14387463
]
Sangjin Lee commented on YARN-3334:
-----------------------------------
Sorry it took a while for me to go over the patch. I just had time now and here
is my initial feedback.
(NodeManager.java)
- l.364: I would prefer the type as interfaces (Map instead of
ConcurrentHashMap)
- getTimelineClient() could initialize multiple timeline client instances for a
given app id (there’s a race): it could be addressed by moving the code that
initializes to after putIfAbsent() succeeds
- l.500: I would also get rid of the containsKey() call; so something like the
following might be good:
{code}
public TimelineClient getTimelineClient(ApplicationId id) {
TimelineClient client = this.timelineClients.get(id);
if (client == null) {
client = TimelineClient.createTimelineClient(id);
TimelineClient old = this.timelineClients.putIfAbsent(id, client);
if (old != null) {
client = old;
} else {
client.init(conf);
client.start();
}
}
return client;
}
{code}
(ApplicationImpl.java)
- I would prefer a method like removeClient() as opposed to returning the
underlying map for modification via getTimelineClients()
(ContainersMonitorImpl.java)
- l.267: it’s usually a good practice to restore the interrupt upon handling an
InterruptedException via Thread.interrupt(), however trivial it may be
(ResourceTrackerService.java)
- Sorry I might be missing something obvious, but why was this change necessary?
> [Event Producers] NM TimelineClient life cycle handling and container metrics
> posting to new timeline service.
> --------------------------------------------------------------------------------------------------------------
>
> 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,
> YARN-3334-v2.patch, YARN-3334-v3.patch
>
>
> After YARN-3039, we have service discovery mechanism to pass app-collector
> service address among collectors, NMs and RM. In this JIRA, we will handle
> service address setting for TimelineClients in NodeManager, and put container
> metrics to the backend storage.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
