[jira] [Commented] (YARN-3210) [Source organization] Refactor timeline aggregator according to new code organization
[ https://issues.apache.org/jira/browse/YARN-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14356272#comment-14356272 ] Zhijie Shen commented on YARN-3210: --- bq. . Yes, it's currently an auxiliary service to the NM, but it can easily be started up as a standalone daemon. I think the idea behind it is that all the node-level aggregator logic should be inside TimelineAggregatorsCollection. In aux service mode, TimelineAggregatorsCollection is started in PerNodeTimelineAggregatorAuxService, while in stand-alone mode, TimelineAggregatorsCollection is started as a separate process. The problem seems to be that the way to start a logic app-level aggregator is not decoupled with the aux service. To make it common no matter the aggregator is in aux service, standalone process or container, starting the app-level aggregator could be treated as a IPC call in Aggregator-NM communication protocol. [Source organization] Refactor timeline aggregator according to new code organization - Key: YARN-3210 URL: https://issues.apache.org/jira/browse/YARN-3210 Project: Hadoop YARN Issue Type: Sub-task Components: timelineserver Reporter: Li Lu Assignee: Li Lu Labels: refactor Fix For: YARN-2928 Attachments: YARN-3210-022715.patch, YARN-3210-030215.patch, YARN-3210-030215_1.patch, YARN-3210-030215_2.patch We may want to refactor the code of timeline aggregator according to the discussion of YARN-3166, the code organization for timeline service v2. We need to refactor the code after we reach an agreement on the aggregator part of YARN-3166. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3210) [Source organization] Refactor timeline aggregator according to new code organization
[ https://issues.apache.org/jira/browse/YARN-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14351113#comment-14351113 ] Sangjin Lee commented on YARN-3210: --- Thanks [~gtCarrera9] for working on this! I know it's too late for a code review, but a couple of comments post-commit: - I'm not too sure if I like the name PerNodeTimelineAggregatorAuxService. Yes, it's currently an auxiliary service to the NM, but it can easily be started up as a standalone daemon. It seems to me that the name appears to suggest the only way to use it is an auxiliary service. Perhaps we can revisit this name later? - minor but TimelineAggregatorWebService has a wildcard import (l.23) [Source organization] Refactor timeline aggregator according to new code organization - Key: YARN-3210 URL: https://issues.apache.org/jira/browse/YARN-3210 Project: Hadoop YARN Issue Type: Sub-task Components: timelineserver Reporter: Li Lu Assignee: Li Lu Labels: refactor Fix For: YARN-2928 Attachments: YARN-3210-022715.patch, YARN-3210-030215.patch, YARN-3210-030215_1.patch, YARN-3210-030215_2.patch We may want to refactor the code of timeline aggregator according to the discussion of YARN-3166, the code organization for timeline service v2. We need to refactor the code after we reach an agreement on the aggregator part of YARN-3166. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3210) [Source organization] Refactor timeline aggregator according to new code organization
[ https://issues.apache.org/jira/browse/YARN-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14343794#comment-14343794 ] Li Lu commented on YARN-3210: - The patch would not apply to normal Jenkins run since Jenkins is based on trunk. I've already tested the patch locally with our distributed shell unit test and it worked locally. [Source organization] Refactor timeline aggregator according to new code organization - Key: YARN-3210 URL: https://issues.apache.org/jira/browse/YARN-3210 Project: Hadoop YARN Issue Type: Sub-task Components: timelineserver Reporter: Li Lu Assignee: Li Lu Labels: refactor Attachments: YARN-3210-022715.patch, YARN-3210-030215.patch We may want to refactor the code of timeline aggregator according to the discussion of YARN-3166, the code organization for timeline service v2. We need to refactor the code after we reach an agreement on the aggregator part of YARN-3166. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3210) [Source organization] Refactor timeline aggregator according to new code organization
[
https://issues.apache.org/jira/browse/YARN-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14343801#comment-14343801
]
Zhijie Shen commented on YARN-3210:
---
Thanks Li! Some additional comments
1. Can we rename the member variable name too? And please double check the
other member variables and local variables whose class name has been changed.
{code}
private final TimelineAggregatorsCollection serviceManager;
{code}
{code}
162 final TimelineAggregatorsCollection serviceManager =
163 (TimelineAggregatorsCollection) context.getAttribute(
164
TimelineAggregatorsCollection.AGGREGATOR_COLLECTION_ATTR_KEY);
{code}
2. Per offline discussion this morning, I thought the collection can hold not
just app-level aggregator, right?
{code}
103 try {
104 service = new AppLevelTimelineAggregator(appId);
105 // initialize, start, and add it to the parent service so it
can be
106 // cleaned up when the parent shuts down
{code}
3. Shall we make the order of super call and other call consistent?
{code}
69@Override
70protected void serviceInit(Configuration conf) throws Exception {
71 serviceManager.init(conf);
72 super.serviceInit(conf);
73}
74
75@Override
76protected void serviceStart() throws Exception {
77 super.serviceStart();
78 serviceManager.start();
79}
80
81@Override
82protected void serviceStop() throws Exception {
83 // stop the service manager
84 serviceManager.stop();
85 super.serviceStop();
86}
{code}
[Source organization] Refactor timeline aggregator according to new code
organization
-
Key: YARN-3210
URL: https://issues.apache.org/jira/browse/YARN-3210
Project: Hadoop YARN
Issue Type: Sub-task
Components: timelineserver
Reporter: Li Lu
Assignee: Li Lu
Labels: refactor
Attachments: YARN-3210-022715.patch, YARN-3210-030215.patch
We may want to refactor the code of timeline aggregator according to the
discussion of YARN-3166, the code organization for timeline service v2. We
need to refactor the code after we reach an agreement on the aggregator part
of YARN-3166.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-3210) [Source organization] Refactor timeline aggregator according to new code organization
[ https://issues.apache.org/jira/browse/YARN-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14344096#comment-14344096 ] Zhijie Shen commented on YARN-3210: --- +1, LGTM [Source organization] Refactor timeline aggregator according to new code organization - Key: YARN-3210 URL: https://issues.apache.org/jira/browse/YARN-3210 Project: Hadoop YARN Issue Type: Sub-task Components: timelineserver Reporter: Li Lu Assignee: Li Lu Labels: refactor Attachments: YARN-3210-022715.patch, YARN-3210-030215.patch, YARN-3210-030215_1.patch We may want to refactor the code of timeline aggregator according to the discussion of YARN-3166, the code organization for timeline service v2. We need to refactor the code after we reach an agreement on the aggregator part of YARN-3166. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3210) [Source organization] Refactor timeline aggregator according to new code organization
[ https://issues.apache.org/jira/browse/YARN-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14343429#comment-14343429 ] Zhijie Shen commented on YARN-3210: --- Thanks for the patch. Two quick comments: 1. Can we remove the build dependency on timeline service in NM module? 2. Shall we call AppLevelTimelineAggregatorsCollection instead of TimelineAggregatorsCollection? Looking into the detail, it's not the collection of any aggregator that extends BaseTimelineAggregator, but the particular AppLevelTimelineAggregator. [Source organization] Refactor timeline aggregator according to new code organization - Key: YARN-3210 URL: https://issues.apache.org/jira/browse/YARN-3210 Project: Hadoop YARN Issue Type: Sub-task Components: timelineserver Reporter: Li Lu Assignee: Li Lu Labels: refactor Attachments: YARN-3210-022715.patch We may want to refactor the code of timeline aggregator according to the discussion of YARN-3166, the code organization for timeline service v2. We need to refactor the code after we reach an agreement on the aggregator part of YARN-3166. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3210) [Source organization] Refactor timeline aggregator according to new code organization
[ https://issues.apache.org/jira/browse/YARN-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14343593#comment-14343593 ] Vinod Kumar Vavilapalli commented on YARN-3210: --- The REST server should be inside TimelineAggregatorsCollection. That way it can be used even for Aggregators-in-a-special container. bq. Shall we call AppLevelTimelineAggregatorsCollection instead of TimelineAggregatorsCollection? Looking into the detail, it's not the collection of any aggregator that extends BaseTimelineAggregator, but the particular AppLevelTimelineAggregator. We can instead make BaseTimelineAggregator as an abstract class and rename it simply to be TimelineAggregator. Then TimelineAggregatorsCollection is a collection of TimelineAggregator objects, as it should be. [Source organization] Refactor timeline aggregator according to new code organization - Key: YARN-3210 URL: https://issues.apache.org/jira/browse/YARN-3210 Project: Hadoop YARN Issue Type: Sub-task Components: timelineserver Reporter: Li Lu Assignee: Li Lu Labels: refactor Attachments: YARN-3210-022715.patch We may want to refactor the code of timeline aggregator according to the discussion of YARN-3166, the code organization for timeline service v2. We need to refactor the code after we reach an agreement on the aggregator part of YARN-3166. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3210) [Source organization] Refactor timeline aggregator according to new code organization
[
https://issues.apache.org/jira/browse/YARN-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14340727#comment-14340727
]
Hadoop QA commented on YARN-3210:
-
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12701435/YARN-3210-022715.patch
against trunk revision 01a1621.
{color:red}-1 patch{color}. The patch command could not apply the patch.
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6786//console
This message is automatically generated.
[Source organization] Refactor timeline aggregator according to new code
organization
-
Key: YARN-3210
URL: https://issues.apache.org/jira/browse/YARN-3210
Project: Hadoop YARN
Issue Type: Sub-task
Components: timelineserver
Reporter: Li Lu
Assignee: Li Lu
Labels: refactor
Attachments: YARN-3210-022715.patch
We may want to refactor the code of timeline aggregator according to the
discussion of YARN-3166, the code organization for timeline service v2. We
need to refactor the code after we reach an agreement on the aggregator part
of YARN-3166.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
