YARN-4984. LogAggregationService shouldn't swallow exception in handling createAppDir() which cause thread leak. (Junping Du via wangda)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/7bd418e4 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/7bd418e4 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/7bd418e4 Branch: refs/heads/HDFS-1312 Commit: 7bd418e48c71590fc8026d69f9b8f8ad42f2aade Parents: e61d431 Author: Wangda Tan <wan...@apache.org> Authored: Wed May 4 11:38:55 2016 -0700 Committer: Wangda Tan <wan...@apache.org> Committed: Wed May 4 11:38:55 2016 -0700 ---------------------------------------------------------------------- .../logaggregation/LogAggregationService.java | 7 +++---- .../logaggregation/TestLogAggregationService.java | 11 ++++++++--- 2 files changed, 11 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/7bd418e4/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/LogAggregationService.java ---------------------------------------------------------------------- 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/LogAggregationService.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/LogAggregationService.java index 2d6b900..d46f7a3 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/LogAggregationService.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/LogAggregationService.java @@ -376,6 +376,9 @@ public class LogAggregationService extends AbstractService implements } else { appDirException = (YarnRuntimeException)e; } + appLogAggregators.remove(appId); + closeFileSystems(userUgi); + throw appDirException; } // TODO Get the user configuration for the list of containers that need log @@ -393,10 +396,6 @@ public class LogAggregationService extends AbstractService implements } }; this.threadPool.execute(aggregatorWrapper); - - if (appDirException != null) { - throw appDirException; - } } protected void closeFileSystems(final UserGroupInformation userUgi) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/7bd418e4/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java index fec12ff..fa9a0b5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java @@ -777,8 +777,8 @@ public class TestLogAggregationService extends BaseContainerManagerTest { dispatcher.await(); ApplicationEvent expectedEvents[] = new ApplicationEvent[]{ - new ApplicationEvent(appId, - ApplicationEventType.APPLICATION_LOG_HANDLING_FAILED) + new ApplicationEvent(appId, + ApplicationEventType.APPLICATION_LOG_HANDLING_FAILED) }; checkEvents(appEventHandler, expectedEvents, false, "getType", "getApplicationID", "getDiagnostic"); @@ -794,10 +794,15 @@ public class TestLogAggregationService extends BaseContainerManagerTest { logAggregationService.stop(); assertEquals(0, logAggregationService.getNumAggregators()); - verify(spyDelSrvc).delete(eq(user), any(Path.class), + // local log dir shouldn't be deleted given log aggregation cannot + // continue due to aggregated log dir creation failure on remoteFS. + verify(spyDelSrvc, never()).delete(eq(user), any(Path.class), Mockito.<Path>anyVararg()); verify(logAggregationService).closeFileSystems( any(UserGroupInformation.class)); + // make sure local log dir is not deleted in case log aggregation + // service cannot be initiated. + assertTrue(appLogDir.exists()); } private void writeContainerLogs(File appLogDir, ContainerId containerId, --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org