[
https://issues.apache.org/jira/browse/YARN-3045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14615275#comment-14615275
]
Junping Du commented on YARN-3045:
----------------------------------
Thanks [~Naganarasimha] for updating the patch!
One of my major question on 005 patch is: why we hook the track of container
start event in ContainerManagerImpl, but for container finished event, we do it
inside of ContainerImpl? We should try to keep NMTimelinePublisher get
referenced in one place if no necessary for other places. IMO, the latter one
sounds like a better choice because we can track more type of container events
(like: ContainerResourceLocalizedEvent, ContainerResourceFailedEvent, etc.)
during container state transition that we are currently missing.
Other minor comments:
In TestDistributedShell.java,
{code}
- @Test(timeout=90000)
+ //@Test(timeout=90000)
{code}
Why do we need to comment this out? We should add back the timeout value here
if no special reason.
In ContainerManagerImpl.java,
{code}
+ private NMTimelinePublisher nmMetricsPublisher;
{code}
We should mark it as final which shouldn't get changed during life cycle of
ContainerManager. The same case for ContainerImpl.java also.
{code}
+ container
+ .getNMTimelinePublisher()
+ .reportContainerResourceUsage(
+ container,
+ currentTime,
+ pId,
+ (currentPmemUsage ==
ResourceCalculatorProcessTree.UNAVAILABLE) ? null
+ : currentPmemUsage,
+ (cpuUsageTotalCoresPercentage ==
ResourceCalculatorProcessTree.UNAVAILABLE) ? null
+ : cpuUsageTotalCoresPercentage);
{code}
No need to transform unavailable value from
ResourceCalculatorProcessTree.UNAVAILABLE to null, as we can check value of
unavailable instead of null later.
In NMTimelinePublisher.java,
{code}
+ protected void handleSystemMetricsEvent(NMTimelineEvent event) {
+ switch (event.getType()) {
+ case CONTAINER_CREATED:
{code}
Indentation between switch and case.
> [Event producers] Implement NM writing container lifecycle events to ATS
> ------------------------------------------------------------------------
>
> Key: YARN-3045
> URL: https://issues.apache.org/jira/browse/YARN-3045
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Reporter: Sangjin Lee
> Assignee: Naganarasimha G R
> Attachments: YARN-3045-YARN-2928.002.patch,
> YARN-3045-YARN-2928.003.patch, YARN-3045-YARN-2928.004.patch,
> YARN-3045-YARN-2928.005.patch, YARN-3045.20150420-1.patch
>
>
> Per design in YARN-2928, implement NM writing container lifecycle events and
> container system metrics to ATS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
