[
https://issues.apache.org/jira/browse/YARN-1389?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13931255#comment-13931255
]
Zhijie Shen commented on YARN-1389:
-----------------------------------
1. Please remove this comment
{code}
/*
* (non-Javadoc)
*
* @see
* org.apache.hadoop.yarn.api.ApplicationClientProtocol#getContainerReport
* (org.apache.hadoop.yarn.api.protocolrecords.GetContainerReportRequest)
{code}
2.
bq. 4. Shouldn't we remove "throw new YarnException("History service is not
enabled.");" in YarnClientImpl?
bq. Actually we should have this if history is not enabled
We'd better not, otherwise, when AHS is disabled, if the object is not found in
RM, we will get this annoying exception. Please follow YOUR previous code
pattern in YarnClientImpl#getApplicationReport
3. Shouldn't you throw ApplicationNotFoundException, as what you did in
getContainerReport()?
{code}
RMApp application = this.rmContext.getRMApps().get(appId);
if (application == null) {
// If the RM doesn't have the application, throw
// ApplicationNotFoundException and let client to handle.
throw new ApplicationAttemptNotFoundException(
"Application Attempt with id '" + appAttemptId
+ "' doesn't exist in RM.");
}
{code}
And in YarnClientImpl, please make the corresponding corresponding changes as
well.
{code}
if (e.getClass() != ContainerNotFoundException.class
&& e.getClass() != ApplicationAttemptNotFoundException.class) {
throw e;
}
{code}
4. Please remove this code. attempts will never be null. It can be empty, but
it's reasonable. If this application even hasn't its first attempt, the list is
empty.
{code}
if (attempts == null) {
throw new ApplicationAttemptNotFoundException(
"ApplicationAttempts not found for " + appId + " Not Found in RM");
}
{code}
5. Similarly, rmContainers can't be null. After YARN-1794, we should have some
walk-around to get the finished containers from RM.
{code}
if(rmContainers == null){
throw new ContainerNotFoundException("Containers for attempt "
+ appAttemptId + " not present");
}
{code}
6. Just return null. Let UI to decide is going to be printed if the diagnostics
is not available.
{code}
- return finishedStatus.getDiagnostics();
+ if (getFinishedStatus() != null) {
+ return getFinishedStatus().getDiagnostics();
+ } else {
+ return "N/A";
+ }
{code}
> ApplicationClientProtocol and ApplicationHistoryProtocol should expose
> analogous 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,
> YARN-1389-4.patch, YARN-1389-5.patch, YARN-1389-6.patch, YARN-1389-7.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)
