[
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)
