Sangjin Lee commented on YARN-2236:

Karthik, the v.6 patch should address all of your comments except #8.

As for #8, it is true that the event handler is bit extraneous. But from the 
code standpoint, it is pretty clean and elegant. We just initialize the 
SharedCacheUploadService, and ContainerImpl can simply publish the event when 
needed. It also makes the coupling between SharedCacheUploadService and 
ContainerImpl loose.

It is possible to have ContainerImpl use SharedCacheUploadService directly, but 
then the SharedCacheUploadService needs to be passed into the ContainerImpl 
constructor so it can be invoked directly. So all in all, I feel that the 
current approach is as clean as the alternative, if not cleaner. Let me know 
your thoughts. Thanks!

> Shared Cache uploader service on the Node Manager
> -------------------------------------------------
>                 Key: YARN-2236
>                 URL: https://issues.apache.org/jira/browse/YARN-2236
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Chris Trezzo
>            Assignee: Chris Trezzo
>         Attachments: YARN-2236-trunk-v1.patch, YARN-2236-trunk-v2.patch, 
> YARN-2236-trunk-v3.patch, YARN-2236-trunk-v4.patch, YARN-2236-trunk-v5.patch, 
> YARN-2236-trunk-v6.patch
> Implement the shared cache uploader service on the node manager.

This message was sent by Atlassian JIRA

Reply via email to