Wangda Tan commented on YARN-4024:

Thanks update, [~zhiguohong] and comments from [~sunilg].

bq. Since we use removeCache, I think update will be better suited as addToCache
+1, I think it's better to rename it to removeFromCache.

bq. Please add more comments and details about interface Resolver and its api. 
I feel the api resolve has to be UnStable and Public for now. may be we can 
separate the interface to another file.
Since it's an internal only interface, I think you should remove public for 
them. And no need to add @Unstable and @public for such internal interfaces. 

bq.  LOG.debug("[" +...
Should be wrapped by isDebugEnabled

Regarding to tests, I think it maybe easier to expose a getResolver (just 
default accessibility is fine) and mark it to be @VisibleForTesting like other 

And it may be important to add a test to make sure DirectResolver will be 
created to avoid future possible regression. 

And I think it's better to modify check {{if (nodeIpCacheTimeout == -1) {}} to 
be <= 0, since it doesn't make sense to have a timeout <= 0 for CachedResolver.


> YARN RM should avoid unnecessary resolving IP when NMs doing heartbeat
> ----------------------------------------------------------------------
>                 Key: YARN-4024
>                 URL: https://issues.apache.org/jira/browse/YARN-4024
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Wangda Tan
>            Assignee: Hong Zhiguo
>         Attachments: YARN-4024-draft-v2.patch, YARN-4024-draft-v3.patch, 
> YARN-4024-draft.patch, YARN-4024-v4.patch
> Currently, YARN RM NodesListManager will resolve IP address every time when 
> node doing heartbeat. When DNS server becomes slow, NM heartbeat will be 
> blocked and cannot make progress.

This message was sent by Atlassian JIRA

Reply via email to