Rohith Sharma K S commented on YARN-5638:

Thanks [~gtCarrera9] for the updating patch. Basically I understand from above 
discussions and attached patch that always consider latest collector address in 
case of multiple collector address has been reported from different NM for same 
application. The overall approach of the patch looks good. 

few comments on the patch, 
h6. AppCollectorData.java
# Can *happensBefore* comparison method name can be changed something 
meaningful ? May we can define comparator method itself.
# stamped --> isStamped?
# In stamped method, I think check for both {{rmIdentifiers && version}}?
# {{public static final long UNSTAMPED_VERSION_NUMBER = -1;}} public --> 

h6. ResourceTrackerService
# line 662, need not have explicit null check for {{previousCollectorData == 

h6. RMApp.java
# I think this interface can have only getter method. Update and remove is 
happens with in the app, these 2 API need not expose as interface.

h6. yarn_server_common_service_protos.proto
# app_collectors_map --> app_collectors_data ?

h6. NodeStatusUpdaterImpl
# In line 966, null check existingData == null not necessarily required.
# In line 962, application has been verified with null before proceeding to add 
collector address. I think else part of this should directly remove from 
{{context.getRegisteringCollectors()}} otherwise leak would occur.

h6. Design comment
# In every NM heartbeat collector address have been sent to RM. RM process 
these data and send back an collector data to NM in response. This happens for 
every heartbeat. Instead of sending for every NM heartbeat, why can't these 
data send through pull mechanism only if collector address has changed? Let 
RMAppImpl trigger event to running nodes, and these nodes can pull collector 
address on heartbeat. 

> Introduce a collector timestamp to uniquely identify collectors creation 
> order in collector discovery
> -----------------------------------------------------------------------------------------------------
>                 Key: YARN-5638
>                 URL: https://issues.apache.org/jira/browse/YARN-5638
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Li Lu
>            Assignee: Li Lu
>         Attachments: YARN-5638-trunk.v1.patch
> As discussed in YARN-3359, we need to further identify timeline collectors' 
> creation order to rebuild collector discovery data in the RM. This JIRA 
> proposes to use <rm_timestamp, logical_version_number> to order collectors 
> for each application in the RM. This timestamp can then be used when a 
> standby RM becomes active and rebuild collector discovery data. 

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to