[4/4] mesos git commit: Docker Containerizer: Used `parentHook` to implement checkpointing.

[jo...@apache.org]

Docker Containerizer: Used `parentHook` to implement checkpointing.

Previously the DockerContainerizerProcess synced explicitly from the
parent process using manual synchronization via a setup function.

Review: https://reviews.apache.org/r/45229/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5e0be20b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5e0be20b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5e0be20b

Branch: refs/heads/master
Commit: 5e0be20b75e41452032fd96d86a85e84f11d4a52
Parents: a2cef18
Author: Joerg Schad <jo...@mesosphere.io>
Authored: Sat Mar 26 12:20:26 2016 +0100
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Sat Mar 26 12:41:45 2016 +0100

----------------------------------------------------------------------
 src/slave/containerizer/docker.cpp | 58 +++++++++++++--------------------
 1 file changed, 23 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/5e0be20b/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 6a8a705..0133628 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -752,18 +752,6 @@ static int setup(const string& directory)
     }
   }
 
-  // Synchronize with parent process by reading a byte from stdin.
-  char c;
-  ssize_t length;
-  while ((length = read(STDIN_FILENO, &c, sizeof(c))) == -1 && errno == EINTR);
-
-  if (length != sizeof(c)) {
-    // This will occur if the slave terminates during executor launch.
-    // There's a reasonable probability this will occur during slave
-    // restarts across a large/busy cluster.
-    ABORT("Failed to synchronize with slave (it has probably exited)");
-  }
-
   return 0;
 }
 
@@ -1210,10 +1198,31 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
         self(),
         [=](const ContainerLogger::SubprocessInfo& subprocessInfo)
           -> Future<pid_t> {
+    // NOTE: The child process will be blocked until all hooks have been
+    // executed.
+    vector<Subprocess::Hook> parentHooks;
+
+    // NOTE: Currently we don't care about the order of the hooks, as
+    // both hooks are independent.
+
+    // A hook that is executed in the parent process. It attempts to checkpoint
+    // the process pid.
+    //
+    // NOTE:
+    // - The child process is blocked by the hook infrastructure while
+    //   these hooks are executed.
+    // - It is safe to bind `this`, as hooks are executed immediately
+    //   in a `subprocess` call.
+    // - If `checkpoiont` returns an Error, the child process will be killed.
+    parentHooks.emplace_back(Subprocess::Hook(lambda::bind(
+        &DockerContainerizerProcess::checkpoint,
+        this,
+        containerId,
+        lambda::_1)));
+
+#ifdef __linux__
     // If we are on systemd, then extend the life of the executor. Any
     // grandchildren's lives will also be extended.
-    std::vector<Subprocess::Hook> parentHooks;
-#ifdef __linux__
     if (systemd::enabled()) {
       parentHooks.emplace_back(Subprocess::Hook(
           &systemd::mesos::extendLifetime));
@@ -1239,27 +1248,6 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
       return Failure("Failed to fork executor: " + s.error());
     }
 
-    // Checkpoint the executor's pid (if necessary).
-    Try<Nothing> checkpointed = checkpoint(containerId, s.get().pid());
-
-    if (checkpointed.isError()) {
-      return Failure(
-          "Failed to checkpoint executor's pid: " + checkpointed.error());
-    }
-
-    // Checkpoing complete, now synchronize with the process so that it
-    // can continue to execute.
-    CHECK_SOME(s.get().in());
-    char c;
-    ssize_t length;
-    while ((length = write(s.get().in().get(), &c, sizeof(c))) == -1 &&
-           errno == EINTR);
-
-    if (length != sizeof(c)) {
-      return Failure("Failed to synchronize with child process: " +
-                     os::strerror(errno));
-    }
-
     return s.get().pid();
   }));
 }