Karthik Kambatla commented on YARN-2183:

bq. we do have a YARN admin command implemented that lets you run the cleaner 
task on demand (YARN-2189). 
Cool. In that case, I see the merit to keeping runCleanerTask around.

bq. this check is needed to prevent a race (i.e. not allow an on-demand run 
when a scheduled run is in progress).
I understand we need a check to prevent the race. I wonder if we can just 
re-use the existing check in CleanerTask#run instead of an explicit check in 
CleanerService#runCleanerTask? From what I remember, that would make the code 
in CleanerTask#run cleaner as well. (no pun)

bq. However, it’s not clear to me whether a dependency from an SCMStore to an 
AppChecker is always a fine requirement for other types of stores.
I poked around a little more, and here is what I think. SharedCacheManager 
creates an instance of AppChecker, rest of the SCM pieces (Store, 
CleanerService) should just use the same instance. This instance can be passed 
either in the constructor or through an SCMContext similar to RMContext. Or, we 
could add SCM#getAppChecker. 

In its current form, CleanerTask#cleanResourceReferences fetches the references 
from the store, checks if the apps are running, and asks the store to remove 
the references. Moving the whole method to the store would simplify the code 

The latest patch looks pretty good but for the above two points. One other nit: 
One of {CleanerTask, CleanerService} has unused imports. 

> Cleaner service for cache manager
> ---------------------------------
>                 Key: YARN-2183
>                 URL: https://issues.apache.org/jira/browse/YARN-2183
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Chris Trezzo
>            Assignee: Chris Trezzo
>         Attachments: YARN-2183-trunk-v1.patch, YARN-2183-trunk-v2.patch, 
> YARN-2183-trunk-v3.patch, YARN-2183-trunk-v4.patch, YARN-2183-trunk-v5.patch
> Implement the cleaner service for the cache manager along with metrics for 
> the service. This service is responsible for cleaning up old resource 
> references in the manager and removing stale entries from the cache.

This message was sent by Atlassian JIRA

Reply via email to