[jira] [Commented] (YARN-987) Adding ApplicationHistoryManager responsible for exposing reports to all clients

["Zhijie Shen (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YARN-987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818636#comment-13818636 ] 

Zhijie Shen commented on YARN-987:
----------------------------------

bq. shouldn't it be using the reader instead of using the whole store interface directly? Also, call the referred object as reader instead of store?
bq. We have only store implementation and we don't have different reader and writer services so by that clients can only use reader service.

IMHO, though we have a single implementation of both reader and writer interfaces, it is still a good coding behavior to use Reader interface only in AHS side, preventing unexpected writing operations by mistake.

In detail, we can do the following to achieve using the Reader interface only:

1. Change ApplicationHistoryManagerImpl to composite service.
2. When the implementation of ApplicationHistoryStore is constructed, use addIfService() to put it as a sub-service of  ApplicationHistoryManagerImpl. This can even avoid the redundant code of calling init, start and stop for  ApplicationHistoryStore.
3. ApplicationHistoryManagerImpl internally only keeps ApplicationHistoryReader interface, which refers to the constructed ApplicationHistoryStore instance.

This is also how YARN-953 is dealing with the issue. Anyway, it's a tiny issue. As the patch has already been checked in, we can refactor it whenever we revisit it.

bq. That's true. I'll try if this can be fixed in another JIRA by having separate Reader and Writer impls.

IMHO, the blocking issue of separating Reader and Writer impls is being aware the same place to read/write the data. In-memory implementation may not be splittable because we don't refer to the common memory address. However, FileSystem implementation is easy to split, because reader/write almost share nothing, and the path of the storage is accessible by both via configuration.

> Adding ApplicationHistoryManager responsible for exposing reports to all clients
> --------------------------------------------------------------------------------
>
>                 Key: YARN-987
>                 URL: https://issues.apache.org/jira/browse/YARN-987
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Mayank Bansal
>            Assignee: Mayank Bansal
>             Fix For: YARN-321
>
>         Attachments: YARN-987-1.patch, YARN-987-2.patch, YARN-987-3.patch, YARN-987-4.patch, YARN-987-5.patch, YARN-987-6.patch, YARN-987-7.patch, YARN-987-8.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.1#6144)