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

Sangjin Lee commented on YARN-3390:
-----------------------------------

I'm very sorry for revisiting this one more time after saying LGTM. :) But it 
occurred to me while looking at your patch.

You brought up a good point that collector.init() and collector.start() can be 
done without synchronization after put() is done (based on the collectorIsNew 
flag). I now realize that it is exactly the same semantics as 
ConcurrentHashMap, and we should be able to do this much more simply using 
ConcurrentHashMap. Sorry for the late change, but I propose this:

{code}
private final ConcurrentMap<ApplicationId,TimelineCollector> collectors = new 
ConcurrentHashMap<>();
...
public TimelineCollector putIfAbsent(ApplicationId appId, TimelineCollector 
collector) {
  TimelineCollector collectorInTable = collectors.putIfAbsent(appId, collector);
  if (collectorInTable == null) {
    LOG.info("the collector for " + appId + " was added");
    collectorInTable = collector;
    // initialize, start, and add it to the collection so it can be
    // cleaned up when the parent shuts down
    collector.init(getConfig());
    collector.start();
    postPut(appId, collectorInTable);
  } else {
    LOG.info("the collector for " + appId + " already exists!");
  }
  return collectorInTable;
}
{code}

Other code remains the same. Essentially the synchronization block in the 
current patch collapses into a single call on CHM.putIfAbsent().

Previously, either [~djp] or [~gtCarrera9] pointed out that there may be a race 
condition in terms of getting different collectors if CHM is used, but looking 
at this, I don't see that happening. Thoughts?

> Reuse TimelineCollectorManager for RM
> -------------------------------------
>
>                 Key: YARN-3390
>                 URL: https://issues.apache.org/jira/browse/YARN-3390
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Zhijie Shen
>            Assignee: Zhijie Shen
>         Attachments: YARN-3390.1.patch, YARN-3390.2.patch, YARN-3390.3.patch
>
>
> RMTimelineCollector should have the context info of each app whose entity  
> has been put



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

Reply via email to