[
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)
