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

Junping Du commented on YARN-3347:
----------------------------------

Thanks [~xgong] for updating the patch! The patch looks good in overall. 
However, I think we need some unit test to cover some core functional methods, 
e.g readContainerLogsForALogType(), etc.
In addition, a question on following code:
{code}
+    if (amContainers.contains("ALL")) {
+      for (AMLogsRequest request : requests) {
+        outputAMContainerLogs(request, conf, appId, logFiles, logCliHelper,
+          appOwner);
+      }
+    }
+
+    for (String amContainer : amContainers) {
+      if (amContainer.equalsIgnoreCase("ALL")) {
+        continue;
+      }
+      int amContainerId = Integer.parseInt(amContainer.trim());
+      if (amContainerId == -1) {
+        outputAMContainerLogs(requests.get(requests.size() - 1), conf, appId,
+          logFiles, logCliHelper, appOwner);
+      } else {
+        if (amContainerId <= requests.size()) {
+          outputAMContainerLogs(requests.get(amContainerId - 1), conf, appId,
+            logFiles, logCliHelper, appOwner);
+        }
+      }
+    }
{code}
My question is: if user specify "ALL" and other attempts in command line, do we 
need to print out other AM attempts log after print all attempts log? From the 
code, it looks to do so but the behavior sounds a little weird. 

> Improve YARN log command to get AMContainer logs as well as running 
> containers logs
> -----------------------------------------------------------------------------------
>
>                 Key: YARN-3347
>                 URL: https://issues.apache.org/jira/browse/YARN-3347
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: log-aggregation
>            Reporter: Xuan Gong
>            Assignee: Xuan Gong
>         Attachments: YARN-3347.1.patch, YARN-3347.1.rebase.patch, 
> YARN-3347.2.patch, YARN-3347.2.rebase.patch, YARN-3347.3.1.patch, 
> YARN-3347.3.patch, YARN-3347.3.rebase.patch
>
>
> Right now, we could specify applicationId, node http address and container ID 
> to get the specify container log. Or we could only specify applicationId to 
> get all the container logs. It is very hard for the users to get logs for AM 
> container since the AMContainer logs have more useful information. Users need 
> to know the AMContainer's container ID and related Node http address.
> We could improve the YARN Log Command to allow users to get AMContainer logs 
> directly



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

Reply via email to