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

Zhijie Shen commented on YARN-967:
----------------------------------

bq. I think we should use same to maintian consistency betwwn ahsclient and 
yarn cli in terms of polling interval. I think keeping lots of confs doesn't 
make sense.

Please remove it, because it doesn't make sense to AHS, it's used by 
YarnClient#submitApplication
{code}
+  @Override
+  protected void serviceInit(Configuration conf) throws Exception {
+    this.ahsAddress = getAHSAddress(conf);
+    statePollIntervalMillis = conf.getLong(
+        YarnConfiguration.YARN_CLIENT_APP_SUBMISSION_POLL_INTERVAL_MS,
+        YarnConfiguration.DEFAULT_YARN_CLIENT_APP_SUBMISSION_POLL_INTERVAL_MS);
+    super.serviceInit(conf);
+  }
{code}

bq. This is on purpose, as we first want to make call to RM and if app is not 
there then call AHS if not there then send exception to client. For attempt and 
contianer it only look into AHS and if not found send exception back to client. 
Thats the older behavior.

The point is:
1. Before the patch, if the application is not found, 
ApplicationNotFoundException is thrown.
2. After the patch, if the application is not found in RM, then check AHS. If 
the application is not found in AHS, return null.

The behavior is changed, such that it is not compatible. I suggest throwing 
ApplicationNotFoundException if the application is not found in AHS as well. It 
seems to be done in the patch of YARN-955. Similar changes should be applied to 
getApplicationAttemptReport, and getContainerReport.

In addition, I also suggest looking the behavior of 
ClientRMService#getApplications, and make ApplicationHistoryClientSerivce to 
behave similarly.

bq. For listapplications we decide not to get info from AHS , we shall do it 
once we will have filters added. We are leaving it for now.

Ok, it's fine. We can fix it later.

More comments:

1. The javadoc is still not fixed
{code}
+   * Prints the application attempt report for an application id.
+   * 
+   * @param applicationId
+   * @throws YarnException
+   */
+  private void printApplicationAttemptReport(String applicationAttemptId)
{code}
{code}
+  /**
+   * Prints the container report for an application attempt id.
+   * 
+   * @param applicationAttemptId
+   * @throws YarnException
+   */
+  private void printContainerReport(String containerId) throws YarnException,
+      IOException {
{code}

2. Then, it's going to reuse the retry policy of RM, which seems not to be 
good. BTW, ContainerManagementProtocolProxy seems not to have the retry policy 
as well. Maybe we should simply create a proxy as HSProxies does?
{code}
+    RetryPolicy retryPolicy = createRetryPolicy(conf);
{code}

> [YARN-321] Command Line Interface(CLI) for Reading Application History 
> Storage Data
> -----------------------------------------------------------------------------------
>
>                 Key: YARN-967
>                 URL: https://issues.apache.org/jira/browse/YARN-967
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Devaraj K
>            Assignee: Mayank Bansal
>         Attachments: YARN-967-1.patch, YARN-967-2.patch, YARN-967-3.patch, 
> YARN-967-4.patch
>
>




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

Reply via email to