Zhijie Shen commented on YARN-1884:

[~xgong], thanks for the patch. Here're some comments:

1. No need to change application_history_server.proto, 
ApplicationHistoryManagerImpl.java, FileSystemApplicationHistoryStore.java, 
MemoryApplicationHistoryStore.java, ContainerFinishData.java, 
ContainerHistoryData.java, ContainerStartData.java, 
ContainerFinishDataPBImpl.java, ContainerStartDataPBImpl.java, 
TestMemoryApplicationHistoryStore.java, RMApplicationHistoryWriter.java, 
TestRMApplicationHistoryWriter.java. It's the deprecated code.

2. Why do we need conf here to compute http or https? getNodeHttpAddress() 
doesn't come with the prefix? If so, we need to fix it in other xxxxblock, CLI 
and webservice too for consistency. For example, when generating the report, we 
should already append the http prefix.
114             container.getNodeHttpAddress() == null ? "#" : WebAppUtils
115               .getHttpSchemePrefix(conf) + container.getNodeHttpAddress(),

3. Is it possible if getContainer() returns null? If so, it will result in NPE. 
Another way is to make getNodeHttpAddress as the method of RMContainer. See how 
we do it for getContainerExitStatus and so on.
              createdTime, container.getContainer().getNodeHttpAddress()));

> ContainerReport should have nodeHttpAddress
> -------------------------------------------
>                 Key: YARN-1884
>                 URL: https://issues.apache.org/jira/browse/YARN-1884
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Zhijie Shen
>            Assignee: Xuan Gong
>         Attachments: YARN-1884.1.patch
> In web UI, we're going to show the node, which used to be to link to the NM 
> web page. However, on AHS web UI, and RM web UI after YARN-1809, the node 
> field has to be set to nodeID where the container is allocated. We need to 
> add nodeHttpAddress to the containerReport to link users to NM web page

This message was sent by Atlassian JIRA

Reply via email to