Sangjin Lee commented on YARN-4129:

Thanks [~Naganarasimha] for the updated patch. Some comments:

- we should remove the obsolete exclude for AbstractTimelineServicePublisher

- l.137: I see we're using {{hashCode()}} in a specific way to ensure all 
events for the same app end up on the same thread. Still, I think we should 
also override {{equals()}} (maybe with appId + eventType) as a good practice.

- why are we removing the license header?

> 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.002.patch, 
> YARN-4129-YARN-2928.003.patch, 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

Reply via email to