hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Junping Du (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-4920) ATS/NM should support a link to dowload/get the logs in text format
Date Thu, 28 Apr 2016 23:56:13 GMT

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

Junping Du commented on YARN-4920:

Quick go through current patch, some comments below:
+    try {
+      containerId = ContainerId.fromString(containerIdStr);
+    } catch (IllegalArgumentException ex) {
+      return createBadResponse(Status.BAD_REQUEST,
+          "Invalid ContainerId: " + containerIdStr);
+    }
Status.BAD_REQUEST/HTTP 400 is usually for request URL is malformed. In this case, it is better
to use Status.NOT_FOUND/HTTP 404 to indicate the resource (containerId) is not found in server
side. Please refer RFC2616 for more details:https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

+      return createBadResponse(Status.BAD_REQUEST,
+          "The application is not at Running or Finished State.");
The same case here. We should use NOT_FOUND instead.

+  private boolean parseBooleanParam(String param) {
+    if (param != null) {
+      return ("true").equalsIgnoreCase(param);
+    }
+    return false;
+  }
We can call {{return ("true").equalsIgnoreCase(param);}} directly which should cover param
is null case.

+  private StreamingOutput getStreamingOutput(ApplicationId appId,
+      String appOwner, final String nodeId, final String containerIdStr,
+      final String logFile) throws IOException{
+	String suffix = LogAggregationUtils.getRemoteNodeLogDirSuffix(conf);
+    org.apache.hadoop.fs.Path remoteRootLogDir = new org.apache.hadoop.fs.Path(
Format (indent) issue here.

    if (appOwner == null) {
      org.apache.hadoop.fs.Path toMatch = LogAggregationUtils
          .getRemoteAppLogDir(remoteRootLogDir, appId, "*", suffix);
      FileStatus[] matching  = fc.util().globStatus(toMatch);
      if (matching == null || matching.length != 1) {
        return null;
      remoteAppDir = matching[0].getPath();
It is possible for an app to match with the other one, like: abcd_011 can also be matched
as abcd_01. Isn't it? If so, we should fix this issue.

            while (true) {
              try {
                byte[] buf = new byte[65535];
We shouldn't allocate memory of buffer inside of while loop which will cause a lot of unncessary
GC operations in case log file is pretty large. Instead, we should reuse the limited size

In NMWebServices.java,
+      if (downloadFile) {
+        response.header("Content-Type", "application/octet-stream");
+        response.header("Content-Disposition", "attachment; filename="
+            + logFile.getName());
+      }
I would suggest to add file's modification time to response.header("Last-Modified", file_modification_time_truncate_to_seconds)
also. User may need to figure out which app's log could be old and which apps' log are new
which shouldn't depend on file's download time via http. However, this is just an optional
comments, see if you want to address now or later.

> ATS/NM should support a link to dowload/get the logs in text format
> -------------------------------------------------------------------
>                 Key: YARN-4920
>                 URL: https://issues.apache.org/jira/browse/YARN-4920
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: yarn
>            Reporter: Xuan Gong
>            Assignee: Xuan Gong
>         Attachments: YARN-4920.20160424.branch-2.patch

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org

View raw message