diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/ContainerImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/ContainerImpl.java index f76e682339d..8031dd6ea40 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/ContainerImpl.java +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/ContainerImpl.java @@ -384,7 +384,7 @@ public ContainerImpl(Configuration conf, Dispatcher dispatcher, UPDATE_DIAGNOSTICS_TRANSITION) .addTransition(ContainerState.SCHEDULED, ContainerState.KILLING, ContainerEventType.KILL_CONTAINER, - new KillBeforeRunningTransition()) + new KillTransition()) .addTransition(ContainerState.SCHEDULED, ContainerState.SCHEDULED, ContainerEventType.UPDATE_CONTAINER_TOKEN, new NotifyContainerSchedulerOfUpdateTransition()) diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java index 04295e13678..23ad408aa98 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java @@ -556,14 +556,10 @@ protected void handleContainerExitCode(int exitCode, Path containerLogDir) { || exitCode == ExitCode.TERMINATED.getExitCode()) { // If the process was killed, Send container_cleanedup_after_kill and // just break out of this method. - - // If Container was killed before starting... NO need to do this. - if (!killedBeforeStart) { - dispatcher.getEventHandler().handle( - new ContainerExitEvent(containerId, - ContainerEventType.CONTAINER_KILLED_ON_REQUEST, exitCode, - diagnosticInfo.toString())); - } + dispatcher.getEventHandler().handle( + new ContainerExitEvent(containerId, + ContainerEventType.CONTAINER_KILLED_ON_REQUEST, exitCode, + diagnosticInfo.toString())); } else if (exitCode != 0) { handleContainerExitWithFailure(containerId, exitCode, containerLogDir, diagnosticInfo); diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainersLauncher.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainersLauncher.java index cfd5d6a95f3..e47a342c9c1 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainersLauncher.java +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainersLauncher.java @@ -23,6 +23,10 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.ExecutorService; + +import org.apache.hadoop.yarn.api.records.ContainerExitStatus; +import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerEventType; +import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerExitEvent; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -151,7 +155,12 @@ public void handle(ContainersLauncherEvent event) { case CLEANUP_CONTAINER_FOR_REINIT: ContainerLaunch launcher = running.remove(containerId); if (launcher == null) { - // Container not launched. So nothing needs to be done. + // Container not launched. + // triggering KILLING to CONTAINER_CLEANEDUP_AFTER_KILL transition. + dispatcher.getEventHandler().handle( + new ContainerExitEvent(containerId, + ContainerEventType.CONTAINER_KILLED_ON_REQUEST, ContainerExitStatus.SUCCESS, + "Container is killed/terminated before being launched.")); return; } diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/TestContainer.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/TestContainer.java index edf26d46dd9..812274d109b 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/TestContainer.java +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/TestContainer.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.refEq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; @@ -664,6 +665,17 @@ public void testKillOnLocalizedWhenContainerNotLaunchedContainerKilled() ContainerLaunch launcher = wc.launcher.running.get(wc.c.getContainerId()); wc.killContainer(); assertEquals(ContainerState.KILLING, wc.c.getContainerState()); + + // check that container cleanup hasn't started at this point. + LocalizationCleanupMatcher cleanupResources = + new LocalizationCleanupMatcher(wc.c); + verify(wc.localizerBus, times(0)).handle(argThat(cleanupResources)); + + // check if containerlauncher cleans up the container launch. + verify(wc.launcherBus) + .handle(refEq(new ContainersLauncherEvent(wc.c, + ContainersLauncherEventType.CLEANUP_CONTAINER), "timestamp")); + launcher.call(); wc.drainDispatcherEvents(); assertEquals(ContainerState.CONTAINER_CLEANEDUP_AFTER_KILL, @@ -676,6 +688,7 @@ public void testKillOnLocalizedWhenContainerNotLaunchedContainerKilled() assertEquals(ContainerState.DONE, wc.c.getContainerState()); assertEquals(killed + 1, metrics.getKilledContainers()); assertEquals(0, metrics.getRunningContainers()); + assertEquals(0, wc.launcher.running.size()); } finally { if (wc != null) { wc.finished(); @@ -1145,7 +1158,7 @@ private void verifyCleanupCall(WrappedContainer wc) throws Exception { ResourcesReleasedMatcher matchesReq = new ResourcesReleasedMatcher(wc.localResources, EnumSet.of( LocalResourceVisibility.PUBLIC, LocalResourceVisibility.PRIVATE, - LocalResourceVisibility.APPLICATION)); + LocalResourceVisibility.APPLICATION), wc.c); verify(wc.localizerBus, atLeastOnce()).handle(argThat(matchesReq)); } @@ -1161,13 +1174,37 @@ private void verifyDockerContainerCleanupCall(WrappedContainer wc) wc.c.getContainerId().toString()))); } - private static class ResourcesReleasedMatcher extends + // Argument matcher for matching container localization cleanup event. + private static class LocalizationCleanupMatcher extends ArgumentMatcher { + LocalizationEventType event; + Container c; + + LocalizationCleanupMatcher(Container c){ + this.event = LocalizationEventType.CLEANUP_CONTAINER_RESOURCES; + this.c = c; + } + + @Override + public boolean matches(Object o) { + if (!(o instanceof ContainerLocalizationCleanupEvent)) { + return false; + } + ContainerLocalizationCleanupEvent evt = + (ContainerLocalizationCleanupEvent) o; + + return (evt.getContainer() == c); + } + } + + private static class ResourcesReleasedMatcher extends + LocalizationCleanupMatcher { final HashSet resources = new HashSet(); ResourcesReleasedMatcher(Map allResources, - EnumSet vis) throws URISyntaxException { + EnumSet vis, Container c) throws URISyntaxException { + super(c); for (Entry e : allResources.entrySet()) { if (vis.contains(e.getValue().getVisibility())) { resources.add(new LocalResourceRequest(e.getValue())); @@ -1177,9 +1214,12 @@ private void verifyDockerContainerCleanupCall(WrappedContainer wc) @Override public boolean matches(Object o) { - if (!(o instanceof ContainerLocalizationCleanupEvent)) { + // match event type and container. + if(!super.matches(o)){ return false; } + + // match resources. ContainerLocalizationCleanupEvent evt = (ContainerLocalizationCleanupEvent) o; final HashSet expected =