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

Naganarasimha G R commented on YARN-1621:
-----------------------------------------

Hi [~noddi]
Sorry for the delayed response was held up with other activties:
* Requires rebase as {{TestYarnCLI.java}} and {{ApplicationCLI.java}} is not 
compiling
* small nits : ContainerCLI,YarnClientImpl, etc : many places line have more 
than 80 chars, may be we can do eclipse formatting once.
* format of Listing of containers for AppAttemptID and AppId can be unified,  
{{writer.println("ApplicationAttempt-Id: " + 
attemptReport.getApplicationAttemptId());}} can be added for  AppAttemptID too 
. ur opinion ?
* code for printing the containers is common for AppAttemptID and AppId, hence 
we can reduce the duplicate code by extracting them to a common method.
* listApplicationContainers can take the converted ApplicationId as argument @ 
ln 160, 
* ApplicationNotFoundException can come even in 
{{client.getContainers(appAttemptId,containerStates)}} better to catch for 
exception and  return error exitCode for listApplicationAttemptContainers too. 
Common try catch block for listApplicationContainers and   capturing 
YarnException, IOException would be good ?
* Exception might become too verbose, Overall was expecting some thing like 
{code}
        String id = cliParser.getOptionValue(LIST_CMD);
        try { 
                  try {
                    
listApplicationContainers(ConverterUtils.toApplicationId(id), containerStates);
                  } catch (IllegalArgumentException e) {
                    try {
                    listApplicationAttemptContainers(
                        ConverterUtils.toApplicationAttemptId(appAttemptId),
                        containerStates);
                    } catch (IllegalArgumentException e) {
                      sysout.println("Wrong format of application ID or 
application attempt ID");
                      return exitCode;
                    }
                  }
         } catch (YarnException e) {
                    return exitCode;
         } catch (IOException e) {
                    return exitCode;
         }
{code}
* Instead of throwing ApplicationNotFoundException we can throw YarnException 
when {{app == null || 
!validApplicationStates.contains(app.getYarnApplicationState())}} in 
listApplicationContainers(applicationId,states)
* Better to comment in YarnClientImpl.getContainers for 
{noformat}isContainerStatesEmpty || !(containerStates.size() == 1
        && containerStates.contains(ContainerState.COMPLETE)){noformat}
* {{Boolean showFinishedContainers}}, better use boolean instead of wrapper 
class
* May be we can leverage the benifit of passing the states to AHS too, this 
will reduce the transfer of
data from AHS to the client. ur opinion ? 
* If we are incorporating the above point then i feel only only when 
appNotFoundInRM we need to query for all states 
from AHS if not querying for COMPLETE state would be sufficient.
* No test cases for modification of 
GetContainersRequestPBImpl/GetContainersRequestProto
* there are some test case failures and findbugs issues reported can you take a 
look at it 

Have not gone through the Test code and applied this patch and tested, once you 
have rebased and we finalized on the above points will check test code and also 
do some verification.

> Add CLI to list rows of <task attempt ID, container ID, host of container, 
> state of container>
> ----------------------------------------------------------------------------------------------
>
>                 Key: YARN-1621
>                 URL: https://issues.apache.org/jira/browse/YARN-1621
>             Project: Hadoop YARN
>          Issue Type: Improvement
>    Affects Versions: 2.2.0
>            Reporter: Tassapol Athiapinya
>            Assignee: Bartosz Ɓugowski
>         Attachments: YARN-1621.1.patch, YARN-1621.2.patch, YARN-1621.3.patch, 
> YARN-1621.4.patch, YARN-1621.5.patch
>
>
> As more applications are moved to YARN, we need generic CLI to list rows of 
> <task attempt ID, container ID, host of container, state of container>. Today 
> if YARN application running in a container does hang, there is no way to find 
> out more info because a user does not know where each attempt is running in.
> For each running application, it is useful to differentiate between 
> running/succeeded/failed/killed containers.
>  
> {code:title=proposed yarn cli}
> $ yarn application -list-containers -applicationId <appId> [-containerState 
> <state of container>]
> where containerState is optional filter to list container in given state only.
> <container state> can be running/succeeded/killed/failed/all.
> A user can specify more than one container state at once e.g. KILLED,FAILED.
> <task attempt ID> <container ID> <host of container> <state of container> 
> {code}
> CLI should work with running application/completed application. If a 
> container runs many task attempts, all attempts should be shown. That will 
> likely be the case of Tez container-reuse application.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to