[jira] [Commented] (YARN-955) [YARN-321] Implementation of ApplicationHistoryProtocol

["Mayank Bansal (JIRA)" <ji...@apache.org>]

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

Mayank Bansal commented on YARN-955:
------------------------------------

Thanks [~zjshen] for the review

bq. 1. Is there any special reason to rename ASHService to ApplicationHistoryClientService?
Its more verbose name and same as other classes.

bq. 2. Inner ApplicationHSClientProtocolHandler is not necessary. ApplicationHistoryClientService can directly implement ApplicationHistoryProtocol, which is what ASHService did before.

I used the same design pattern used in Job History server. And moreover its more cleaner design then having service derived from everything. Secondly you have multiple protocols implementation.

bq. 3. Incorrect log bellow:
Done.

bq. 4. We should use the newInstance method from the record class for GetApplicationAttemptReportResponse and all the other records.
Done.

bq. 5. Some methods missed @Override, for example
They are not override methods, those are helper functions.

bq. 6. The two methods bellow is not implemented, but we can do it separately, because we need to implement a DelegationTokenSecretManager first.
Those will be implemented once we implement security.

bq. 7. Did you miss ApplicationHistoryContext in the patch or is it included in the patch of other Jira?
History Context is part of YARN-987.

bq. 8. Why the method bellow has the default access control?
Used in Test.

bq. 9. In RM and NM, we usually add a protected createXXXX() method for a sub service, such that we can override it, and change to another implementation. It is convenient when we want to mock some part of AHS when drafting the test cases.
Done.

bq. 10. Shall we have the test cases for the ApplicationHistoryProtocol implementation?
Done.

> [YARN-321] Implementation of ApplicationHistoryProtocol
> -------------------------------------------------------
>
>                 Key: YARN-955
>                 URL: https://issues.apache.org/jira/browse/YARN-955
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Vinod Kumar Vavilapalli
>            Assignee: Mayank Bansal
>         Attachments: YARN-955-1.patch, YARN-955-2.patch
>
>




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