[
https://issues.apache.org/jira/browse/YARN-4129?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14951458#comment-14951458
]
Sangjin Lee commented on YARN-4129:
-----------------------------------
Sorry [~Naganarasimha] it took me a while to review this.
I'm generally +1 on the effort here to reduce unnecessary layers (getting rid
of event types) and additional flexibility you mentioned.
I know also there is a discussion on whether we should set createdTime,
modifiedTime, etc. on the entities themselves (forgot the JIRA id), and it has
some implications on this. I will chime in there later, but IMO it'd be good to
set things like createdTime directly on the entities to have consistent and
uniform access to those important times. We can make those changes (if we
agree) in that JIRA, though.
(ResourceManager.java)
- l.396: the service is being added twice (another in l.275); I would say
remove l.396
- l.514: I'm slightly confused that (apart from l.396) the publisher is
registered once to the RM itself and another time here to the RMActiveServices.
Is it needed? How would the service stop work (since these are composite
services)?
(SystemMetricsPublisher.java)
- l.27: nit: space before the brace
(TimelineServiceV2Publisher.java)
- l.80: normally we call {{super.serviceStart()}} at the end rather than at the
beginning, right?
- l.155: Are you referring to the issue of having the app level timeline
collector hanging around to process late-coming writes? If so, we should add
the JIRA id here in the comment so we that we can keep track. If not, could you
please explain the TODO comment here?
> Refactor the SystemMetricPublisher in RM to better support newer events
> -----------------------------------------------------------------------
>
> Key: YARN-4129
> URL: https://issues.apache.org/jira/browse/YARN-4129
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Naganarasimha G R
> Assignee: Naganarasimha G R
> Attachments: YARN-4129.YARN-2928.001.patch
>
>
> Currently to add new timeline event/ entity in RM side, one has to add a
> method in publisher and a method in handler and create a new event class
> which looks cumbersome and redundant. also further all the events might not
> be required to be published in V1 & V2. So adopting the approach similar to
> what was adopted in YARN-3045(NM side)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
