[jira] [Commented] (YARN-4545) Allow YARN distributed shell to use ATS v1.5 APIs

[Junping Du (JIRA)]

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

Junping Du commented on YARN-4545:
----------------------------------

Thanks [~gtCarrera9] for updating the patch! 
My comments:
In YarnConfiguration.java,
{code}
+   * @return whether the timeline service v.1.5 is enabled. V.1.5 refers to a
+   * version greater than equal to 1.5.
{code}
Here is a bit misleading, "version greater than equal to 1.5" conflicts with 
implementation: {{Math.abs(getTimelineServiceVersion(conf) - 1.5) < 0.00001;}}, 
we need more correct description here.


In DistributedShellTimelinePlugin.java,
{{toEntitiGroupId(String strAppId)}} is better to be {{toEntityGroupId}}

{code}
if (entityType.equals(ApplicationMaster.DSEntity.DS_CONTAINER.toString())) {
{code}
Better to be {{if 
(ApplicationMaster.DSEntity.DS_CONTAINER.toString().equals(entityType))}} to 
get rid of NPE.

{code}
+  public Set<TimelineEntityGroupId> getTimelineEntityGroupId(String entityType,
+      SortedSet<String> entityIds, Set<String> eventTypes) {
+    return null;
+  }
{code}
Why don't we implement anything here?

In ContainerLaunchFailAppMaster.java,
{code}
     public FailContainerLaunchNMCallbackHandler(
-      ApplicationMaster applicationMaster) {
-      super(applicationMaster);
+      ApplicationMaster applicationMaster, Configuration config) {
+      super(applicationMaster, config);
     }
{code}
It sounds like config is not necessary as applicationMaster should have config 
itself. 

> Allow YARN distributed shell to use ATS v1.5 APIs
> -------------------------------------------------
>
>                 Key: YARN-4545
>                 URL: https://issues.apache.org/jira/browse/YARN-4545
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Li Lu
>            Assignee: Li Lu
>         Attachments: YARN-4545-YARN-4265.001.patch, 
> YARN-4545-trunk.001.patch, YARN-4545-trunk.002.patch, 
> YARN-4545-trunk.003.patch
>
>
> We can use YARN distributed shell as a demo for the ATS v1.5 APIs. We need to 
> allow distributed shell post data with ATS v1.5 API if 1.5 is enabled in the 
> system. We also need to provide a sample plugin to read those data out. 



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