diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index 4389e277c9f..3620a718a74 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -598,6 +598,9 @@ Release 2.7.2 - UNRELEASED BUG FIXES + YARN-3793. Several NPEs when deleting local files on NM recovery (Varun + Saxena via jlowe) + Release 2.7.1 - UNRELEASED INCOMPATIBLE CHANGES 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 4b95a03307a..654eb0b1a52 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 @@ -276,10 +276,10 @@ public class AppLogAggregatorImpl implements AppLogAggregator { aggregator.doContainerLogAggregation(writer, appFinished); if (uploadedFilePathsInThisCycle.size() > 0) { uploadedLogsInThisCycle = true; + this.delService.delete(this.userUgi.getShortUserName(), null, + uploadedFilePathsInThisCycle + .toArray(new Path[uploadedFilePathsInThisCycle.size()])); } - this.delService.delete(this.userUgi.getShortUserName(), null, - uploadedFilePathsInThisCycle - .toArray(new Path[uploadedFilePathsInThisCycle.size()])); // This container is finished, and all its logs have been uploaded, // remove it from containerLogAggregators. 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 fd97cef9ff2..6a3d270eac0 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 @@ -31,6 +31,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -280,6 +281,41 @@ public class TestLogAggregationService extends BaseContainerManagerTest { verifyLocalFileDeletion(logAggregationService); } + /* Test to verify fix for YARN-3793 */ + @Test + public void testNoLogsUploadedOnAppFinish() throws Exception { + this.delSrvc = new DeletionService(createContainerExecutor()); + delSrvc = spy(delSrvc); + this.delSrvc.init(conf); + this.conf.set(YarnConfiguration.NM_LOG_DIRS, localLogDir.getAbsolutePath()); + this.conf.set(YarnConfiguration.NM_REMOTE_APP_LOG_DIR, + this.remoteRootLogDir.getAbsolutePath()); + + LogAggregationService logAggregationService = new LogAggregationService( + dispatcher, this.context, this.delSrvc, super.dirsHandler); + logAggregationService.init(this.conf); + logAggregationService.start(); + + ApplicationId app = BuilderUtils.newApplicationId(1234, 1); + File appLogDir = new File(localLogDir, ConverterUtils.toString(app)); + appLogDir.mkdir(); + LogAggregationContext context = + LogAggregationContext.newInstance("HOST*", "sys*"); + logAggregationService.handle(new LogHandlerAppStartedEvent(app, this.user, + null, ContainerLogsRetentionPolicy.ALL_CONTAINERS, this.acls, context)); + + ApplicationAttemptId appAttemptId = + BuilderUtils.newApplicationAttemptId(app, 1); + ContainerId cont = BuilderUtils.newContainerId(appAttemptId, 1); + writeContainerLogs(appLogDir, cont, new String[] { "stdout", + "stderr", "syslog" }); + logAggregationService.handle(new LogHandlerContainerFinishedEvent(cont, 0)); + logAggregationService.handle(new LogHandlerAppFinishedEvent(app)); + logAggregationService.stop(); + delSrvc.stop(); + // Aggregated logs should not be deleted if not uploaded. + verify(delSrvc, times(0)).delete(user, null); + } @Test public void testNoContainerOnNode() throws Exception {