From 5a1b33507b935f91d6dee6056fe840e778fb198e Mon Sep 17 00:00:00 2001 From: Vinod Kumar Vavilapalli Date: Mon, 16 Dec 2013 19:27:48 +0000 Subject: [PATCH] YARN-1145. Fixed a potential file-handle leak in the web interface for displaying aggregated logs. Contributed by Rohith Sharma. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1551326 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-yarn-project/CHANGES.txt | 3 + .../logaggregation/AggregatedLogFormat.java | 8 +- .../yarn/webapp/log/AggregatedLogsBlock.java | 192 +++++++++--------- .../TestAggregatedLogFormat.java | 4 +- .../TestAggregatedLogsBlock.java | 2 +- .../logaggregation/AppLogAggregatorImpl.java | 2 +- 6 files changed, 109 insertions(+), 102 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index e03de63ec4..07c9711f9d 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -249,6 +249,9 @@ Release 2.4.0 - UNRELEASED YARN-1505. Fixed Webapplication proxy server to not hardcode its bind address. (Xuan Gong via vinodkv) + YARN-1145. Fixed a potential file-handle leak in the web interface for + displaying aggregated logs. (Rohith Sharma via vinodkv) + Release 2.3.0 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java index ded6058c56..6ec4ec2ed4 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java @@ -53,6 +53,7 @@ import org.apache.hadoop.fs.Options; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; +import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.io.SecureIOUtils; import org.apache.hadoop.io.Writable; import org.apache.hadoop.io.file.tfile.TFile; @@ -294,7 +295,7 @@ public void append(LogKey logKey, LogValue logValue) throws IOException { out.close(); } - public void closeWriter() { + public void close() { try { this.writer.close(); } catch (IOException e) { @@ -569,9 +570,8 @@ public static void readAContainerLogsForALogType( out.println(""); } - public void close() throws IOException { - this.scanner.close(); - this.fsDataIStream.close(); + public void close() { + IOUtils.cleanup(LOG, scanner, reader, fsDataIStream); } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/log/AggregatedLogsBlock.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/log/AggregatedLogsBlock.java index da69d2fb21..2b83e6941e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/log/AggregatedLogsBlock.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/log/AggregatedLogsBlock.java @@ -59,109 +59,113 @@ public class AggregatedLogsBlock extends HtmlBlock { @Override protected void render(Block html) { - ContainerId containerId = verifyAndGetContainerId(html); - NodeId nodeId = verifyAndGetNodeId(html); - String appOwner = verifyAndGetAppOwner(html); - LogLimits logLimits = verifyAndGetLogLimits(html); - if (containerId == null || nodeId == null || appOwner == null - || appOwner.isEmpty() || logLimits == null) { - return; - } - - ApplicationId applicationId = - containerId.getApplicationAttemptId().getApplicationId(); - String logEntity = $(ENTITY_STRING); - if (logEntity == null || logEntity.isEmpty()) { - logEntity = containerId.toString(); - } - - if (!conf.getBoolean(YarnConfiguration.LOG_AGGREGATION_ENABLED, - YarnConfiguration.DEFAULT_LOG_AGGREGATION_ENABLED)) { - html.h1() - ._("Aggregation is not enabled. Try the nodemanager at " + nodeId) - ._(); - return; - } - - Path remoteRootLogDir = - new Path(conf.get(YarnConfiguration.NM_REMOTE_APP_LOG_DIR, - YarnConfiguration.DEFAULT_NM_REMOTE_APP_LOG_DIR)); AggregatedLogFormat.LogReader reader = null; try { - reader = - new AggregatedLogFormat.LogReader(conf, - LogAggregationUtils.getRemoteNodeLogFileForApp( - remoteRootLogDir, applicationId, appOwner, nodeId, - LogAggregationUtils.getRemoteNodeLogDirSuffix(conf))); - } catch (FileNotFoundException e) { - // ACLs not available till the log file is opened. - html.h1() - ._("Logs not available for " - + logEntity - + ". Aggregation may not be complete, " - + "Check back later or try the nodemanager at " - + nodeId)._(); - return; - } catch (IOException e) { - html.h1()._("Error getting logs for " + logEntity)._(); - LOG.error("Error getting logs for " + logEntity, e); - return; - } - - String owner = null; - Map appAcls = null; - try { - owner = reader.getApplicationOwner(); - appAcls = reader.getApplicationAcls(); - } catch (IOException e) { - html.h1()._("Error getting logs for " + logEntity)._(); - LOG.error("Error getting logs for " + logEntity, e); - return; - } - ApplicationACLsManager aclsManager = new ApplicationACLsManager(conf); - aclsManager.addApplication(applicationId, appAcls); - - String remoteUser = request().getRemoteUser(); - UserGroupInformation callerUGI = null; - if (remoteUser != null) { - callerUGI = UserGroupInformation.createRemoteUser(remoteUser); - } - if (callerUGI != null - && !aclsManager.checkAccess(callerUGI, ApplicationAccessType.VIEW_APP, - owner, applicationId)) { - html.h1() - ._("User [" + remoteUser - + "] is not authorized to view the logs for " + logEntity)._(); - return; - } - - String desiredLogType = $(CONTAINER_LOG_TYPE); - try { - AggregatedLogFormat.ContainerLogsReader logReader = - reader.getContainerLogsReader(containerId); - if (logReader == null) { - html.h1()._( - "Logs not available for " + logEntity - + ". Could be caused by the rentention policy")._(); + ContainerId containerId = verifyAndGetContainerId(html); + NodeId nodeId = verifyAndGetNodeId(html); + String appOwner = verifyAndGetAppOwner(html); + LogLimits logLimits = verifyAndGetLogLimits(html); + if (containerId == null || nodeId == null || appOwner == null + || appOwner.isEmpty() || logLimits == null) { return; } - boolean foundLog = readContainerLogs(html, logReader, logLimits, - desiredLogType); + ApplicationId applicationId = containerId.getApplicationAttemptId() + .getApplicationId(); + String logEntity = $(ENTITY_STRING); + if (logEntity == null || logEntity.isEmpty()) { + logEntity = containerId.toString(); + } - if (!foundLog) { - if (desiredLogType.isEmpty()) { - html.h1("No logs available for container " + containerId.toString()); - } else { - html.h1("Unable to locate '" + desiredLogType - + "' log for container " + containerId.toString()); + if (!conf.getBoolean(YarnConfiguration.LOG_AGGREGATION_ENABLED, + YarnConfiguration.DEFAULT_LOG_AGGREGATION_ENABLED)) { + html.h1() + ._("Aggregation is not enabled. Try the nodemanager at " + nodeId) + ._(); + return; + } + + Path remoteRootLogDir = new Path(conf.get( + YarnConfiguration.NM_REMOTE_APP_LOG_DIR, + YarnConfiguration.DEFAULT_NM_REMOTE_APP_LOG_DIR)); + + try { + reader = new AggregatedLogFormat.LogReader(conf, + LogAggregationUtils.getRemoteNodeLogFileForApp(remoteRootLogDir, + applicationId, appOwner, nodeId, + LogAggregationUtils.getRemoteNodeLogDirSuffix(conf))); + } catch (FileNotFoundException e) { + // ACLs not available till the log file is opened. + html.h1() + ._("Logs not available for " + logEntity + + ". Aggregation may not be complete, " + + "Check back later or try the nodemanager at " + nodeId)._(); + return; + } catch (IOException e) { + html.h1()._("Error getting logs for " + logEntity)._(); + LOG.error("Error getting logs for " + logEntity, e); + return; + } + + String owner = null; + Map appAcls = null; + try { + owner = reader.getApplicationOwner(); + appAcls = reader.getApplicationAcls(); + } catch (IOException e) { + html.h1()._("Error getting logs for " + logEntity)._(); + LOG.error("Error getting logs for " + logEntity, e); + return; + } + ApplicationACLsManager aclsManager = new ApplicationACLsManager(conf); + aclsManager.addApplication(applicationId, appAcls); + + String remoteUser = request().getRemoteUser(); + UserGroupInformation callerUGI = null; + if (remoteUser != null) { + callerUGI = UserGroupInformation.createRemoteUser(remoteUser); + } + if (callerUGI != null + && !aclsManager.checkAccess(callerUGI, + ApplicationAccessType.VIEW_APP, owner, applicationId)) { + html.h1() + ._("User [" + remoteUser + + "] is not authorized to view the logs for " + logEntity)._(); + return; + } + + String desiredLogType = $(CONTAINER_LOG_TYPE); + try { + AggregatedLogFormat.ContainerLogsReader logReader = reader + .getContainerLogsReader(containerId); + if (logReader == null) { + html.h1() + ._("Logs not available for " + logEntity + + ". Could be caused by the rentention policy")._(); + return; } + + boolean foundLog = readContainerLogs(html, logReader, logLimits, + desiredLogType); + + if (!foundLog) { + if (desiredLogType.isEmpty()) { + html.h1("No logs available for container " + containerId.toString()); + } else { + html.h1("Unable to locate '" + desiredLogType + + "' log for container " + containerId.toString()); + } + return; + } + } catch (IOException e) { + html.h1()._("Error getting logs for " + logEntity)._(); + LOG.error("Error getting logs for " + logEntity, e); return; } - } catch (IOException e) { - html.h1()._("Error getting logs for " + logEntity)._(); - LOG.error("Error getting logs for " + logEntity, e); - return; + } finally { + if (reader != null) { + reader.close(); + } } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogFormat.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogFormat.java index 49d8b2d732..f8f425c118 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogFormat.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogFormat.java @@ -114,7 +114,7 @@ public void testReadAcontainerLogs1() throws Exception { testContainerId, ugi.getShortUserName()); logWriter.append(logKey, logValue); - logWriter.closeWriter(); + logWriter.close(); // make sure permission are correct on the file FileStatus fsStatus = fs.getFileStatus(remoteAppLogFile); @@ -194,7 +194,7 @@ public void testContainerLogsFileAccess() throws IOException { ugi.getShortUserName()); logWriter.append(logKey, logValue); - logWriter.closeWriter(); + logWriter.close(); BufferedReader in = new BufferedReader(new FileReader(new File(remoteAppLogFile diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogsBlock.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogsBlock.java index 3c720c4888..94902d43f6 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogsBlock.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogsBlock.java @@ -229,7 +229,7 @@ private void writeLog(Configuration configuration, String user) writer.append(new AggregatedLogFormat.LogKey("container_0_0001_01_000001"), new AggregatedLogFormat.LogValue(rootLogDirs, containerId,UserGroupInformation.getCurrentUser().getShortUserName())); - writer.closeWriter(); + writer.close(); } private void writeLogs(String dirName) throws Exception { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AppLogAggregatorImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AppLogAggregatorImpl.java index 6ef794442c..805b9e0b1b 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AppLogAggregatorImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AppLogAggregatorImpl.java @@ -178,7 +178,7 @@ private void doAppLogAggregation() { localAppLogDirs); if (this.writer != null) { - this.writer.closeWriter(); + this.writer.close(); LOG.info("Finished aggregate log-file for app " + this.applicationId); }