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

Zhijie Shen commented on YARN-1389:
-----------------------------------

Thanks for the new patch. Here're some more comments.

1. I still see "<code>ApplicationHistoryServer</code>" in 
ApplicationClientProtocol. And some description sounds not accurate. For 
example,
{code}
+   * <p>
+   * The interface used by clients to get a report of all Application attempts
+   * in the cluster from the <code>ApplicationHistoryServer</code>.
+   * </p>
{code}
 Please double check the javadoc

2. ApplicationHistoryProtocol's javadoc has been wrongly modified.

3. Is it better to simplify the following condition? Same for all the similar 
conditions in the patch
{code}
+      if (!((e.getClass() == ApplicationNotFoundException.class) || (e
+          .getClass() == ApplicationAttemptNotFoundException.class))) {
{code}
to
{code}
+      if (e.getClass() != ApplicationNotFoundException.class && e
+          .getClass() != ApplicationAttemptNotFoundException.class) {
{code}

4. Please match the XXXXNotFoundException that will be thrown in 
ClientRMService, and that is analyzed in YarnClientImpl.

5. It is still an in-progress patch, isn't it? The test cases are still missing.

bq. 4. Users are not able to get completed application list via YarnClient
bq. Done

Didn't see the change to allow user to get the application list from the history

bq. These are just utility functions, do you think they are needed in 
RMAPPATtempt and RMContainer?

Please see what RMApp does



> ApplicationClientProtocol and ApplicationHistoryProtocol should expose analog 
> APIs
> ----------------------------------------------------------------------------------
>
>                 Key: YARN-1389
>                 URL: https://issues.apache.org/jira/browse/YARN-1389
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Mayank Bansal
>            Assignee: Mayank Bansal
>         Attachments: YARN-1389-1.patch, YARN-1389-2.patch, YARN-1389-3.patch
>
>
> As we plan to have the APIs in ApplicationHistoryProtocol to expose the 
> reports of *finished* application attempts and containers, we should do the 
> same for ApplicationClientProtocol, which will return the reports of 
> *running* attempts and containers.
> Later on, we can improve YarnClient to direct the query of running instance 
> to ApplicationClientProtocol, while that of finished instance to 
> ApplicationHistoryProtocol, making it transparent to the users.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to