[
https://issues.apache.org/jira/browse/YARN-4058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14707818#comment-14707818
]
Sangjin Lee commented on YARN-4058:
-----------------------------------
I see. We might then want to address it on our branch. I would still be
reluctant to touch {{ApplicationACLsManager}} or even the constructor. We would
end up touching quite a few files just to do that clean-up. For that issue, I'd
raise it for the trunk and have it fixed there.
As for the application creation issue, I have one small nit, and one larger
question to discuss. As for the nit,
{code}
901 if (null ==
context.getApplications().putIfAbsent(applicationID,
902 application)) {
{code}
This style (having values like null first and then the expression) is a
carry-over from the C-style defensive coding, and is no longer needed in java.
I would just do the normal style:
{code}
901 if (context.getApplications().putIfAbsent(applicationID,
902 application) == null) {
{code}
I know it's existing code, but it'd be good to change that.
The larger question is this. The timeline client is being created *and started*
as part of the {{ApplicationImpl}} constructor call. However, it seems like
we're not stopping the timeline client anywhere. Right now, it might not be a
big issue as the timeline client isn't doing anything special during the
service start, but what if we create things like thread pools as part of the
timeline client as you suggested? Then the timeline client must be stopped
properly as part of lifecycle management. It's not clear how we can do that if
they are created inside {{ApplicationImpl}}. Any thoughts on that?
> Miscellaneous issues in NodeManager project
> -------------------------------------------
>
> Key: YARN-4058
> URL: https://issues.apache.org/jira/browse/YARN-4058
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Reporter: Naganarasimha G R
> Assignee: Naganarasimha G R
> Priority: Minor
> Attachments: YARN-4058.YARN-2928.001.patch
>
>
> # TestSystemMetricsPublisherForV2.testPublishApplicationMetrics is failing
> # Unused ApplicationACLsManager in ContainerManagerImpl
> # In ContainerManagerImpl.startContainerInternal ApplicationImpl instance is
> created and then checked whether it exists in context.getApplications().
> everytime ApplicationImpl is created state machine is intialized and
> TimelineClient is created which is required only if added to the context.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
