hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Junping Du (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-3045) [Event producers] Implement NM writing container lifecycle events to ATS
Date Mon, 06 Jul 2015 16:32:05 GMT

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

Mime
View raw message