[hadoop] branch trunk updated: YARN-9486. Docker container exited with failure does not get clean up correctly. Contributed by Eric Yang

[eb...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ebadger pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 79d3d35  YARN-9486. Docker container exited with failure does not get clean up correctly. Contributed by Eric Yang
79d3d35 is described below

commit 79d3d35398cb5348cfd62e41e3318ec7a337421a
Author: Eric Badger <eb...@verizonmedia.com>
AuthorDate: Fri Apr 26 01:21:28 2019 +0000

    YARN-9486. Docker container exited with failure does not get clean up correctly. Contributed by Eric Yang
---
 .../containermanager/launcher/ContainerCleanup.java         |  6 ++++--
 .../containermanager/launcher/ContainerRelaunch.java        |  9 ++++++++-
 .../containermanager/launcher/TestContainerCleanup.java     | 13 +++++++++++++
 3 files changed, 25 insertions(+), 3 deletions(-)

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/launcher/ContainerCleanup.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerCleanup.java
index b63becf..faf926a 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerCleanup.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerCleanup.java
@@ -95,8 +95,10 @@ public class ContainerCleanup implements Runnable {
           + " killed in store", e);
     }
 
-    // launch flag will be set to true if process already launched
-    boolean alreadyLaunched = !launch.markLaunched();
+    // launch flag will be set to true if process already launched,
+    // in process of launching, or failed to launch.
+    boolean alreadyLaunched = !launch.markLaunched() ||
+        launch.isLaunchCompleted();
     if (!alreadyLaunched) {
       LOG.info("Container " + containerIdStr + " not launched."
           + " No cleanup needed to be done");
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/launcher/ContainerRelaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerRelaunch.java
index 226b53d..f165605 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerRelaunch.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerRelaunch.java
@@ -87,7 +87,14 @@ public class ContainerRelaunch extends ContainerLaunch {
       Path nmPrivateTruststorePath = (container.getCredentials().getSecretKey(
           AMSecretKeys.YARN_APPLICATION_AM_TRUSTSTORE) == null) ? null :
           getNmPrivateTruststorePath(appIdStr, containerIdStr);
-      pidFilePath = getPidFilePath(appIdStr, containerIdStr);
+      try {
+        // try to locate existing pid file.
+        pidFilePath = getPidFilePath(appIdStr, containerIdStr);
+      } catch (IOException e) {
+        // reset pid file path if it did not exist.
+        String pidFileSubpath = getPidFileSubpath(appIdStr, containerIdStr);
+        pidFilePath = dirsHandler.getLocalPathForWrite(pidFileSubpath);
+      }
 
       LOG.info("Relaunch container with "
           + "workDir = " + containerWorkDir.toString()
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/launcher/TestContainerCleanup.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerCleanup.java
index 6c99379..17574df 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerCleanup.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerCleanup.java
@@ -79,6 +79,7 @@ public class TestContainerCleanup {
 
     launch = mock(ContainerLaunch.class);
     launch.containerAlreadyLaunched = new AtomicBoolean(false);
+    launch.completed = new AtomicBoolean(false);
 
     launch.pidFilePath = new Path("target/" + containerId.toString() + ".pid");
     when(launch.getContainerPid()).thenReturn(containerId.toString());
@@ -105,4 +106,16 @@ public class TestContainerCleanup {
     Assert.assertEquals("signal", ContainerExecutor.Signal.TERM,
         captor.getValue().getSignal());
   }
+
+  @Test
+  public void testFailedExitCleanup() throws Exception {
+    launch.completed.set(true);
+    cleanup.run();
+    ArgumentCaptor<ContainerSignalContext> captor =
+        ArgumentCaptor.forClass(ContainerSignalContext.class);
+
+    verify(executor, Mockito.times(1)).signalContainer(captor.capture());
+    Assert.assertEquals("signal", ContainerExecutor.Signal.TERM,
+        captor.getValue().getSignal());
+  }
 }


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