You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2013/08/14 20:07:39 UTC

[16/18] git commit: Fixed slave to not re-register with Command Executors.

Fixed slave to not re-register with Command Executors.

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


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

Branch: refs/heads/master
Commit: 7ca371ce4ca33f2a189037b57e091c6ad0a5aa48
Parents: 755308d
Author: Vinod Kone <vi...@twitter.com>
Authored: Sun Aug 11 18:56:15 2013 -0700
Committer: Vinod Kone <vi...@twitter.com>
Committed: Wed Aug 14 10:57:42 2013 -0700

----------------------------------------------------------------------
 src/slave/slave.cpp | 44 +++++++++++++++++++++-----------------------
 src/slave/slave.hpp |  2 ++
 2 files changed, 23 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7ca371ce/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 11e4826..803da8d 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -705,13 +705,18 @@ void Slave::doReliableRegistration()
           continue;
         }
 
-        // TODO(bmahler): Kill this in 0.15.0, as in 0.14.0 we've
-        // added code into the Scheduler Driver to ensure the
-        // framework id is set in ExecutorInfo, effectively making
-        // it a required field.
-        ExecutorInfo* executorInfo = message.add_executor_infos();
-        executorInfo->MergeFrom(executor->info);
-        executorInfo->mutable_framework_id()->MergeFrom(framework->id);
+        // Do not re-register with Command Executors because the
+        // master doesn't store them; they are generated by the slave.
+        if (!executor->commandExecutor) {
+          ExecutorInfo* executorInfo = message.add_executor_infos();
+          executorInfo->MergeFrom(executor->info);
+
+          // TODO(bmahler): Kill this in 0.15.0, as in 0.14.0 we've
+          // added code into the Scheduler Driver to ensure the
+          // framework id is set in ExecutorInfo, effectively making
+          // it a required field.
+          executorInfo->mutable_framework_id()->MergeFrom(framework->id);
+        }
 
         // Add launched tasks.
         // TODO(vinod): Use foreachvalue instead once LinkedHashmap
@@ -2135,8 +2140,6 @@ void Slave::executorTerminated(
       monitor.unwatch(frameworkId, executorId)
         .onAny(lambda::bind(_unwatch, lambda::_1, frameworkId, executorId));
 
-      Option<bool> isCommandExecutor;
-
       // Transition all live tasks to TASK_LOST/TASK_FAILED.
       // If the isolator destroyed the executor (e.g., due to OOM event)
       // or if this is a command executor, we send TASK_FAILED status updates
@@ -2155,8 +2158,7 @@ void Slave::executorTerminated(
         foreach (Task* task, executor->launchedTasks.values()) {
           if (!protobuf::isTerminalState(task->state())) {
             mesos::TaskState taskState;
-            isCommandExecutor = !task->has_executor_id();
-            if (destroyed || isCommandExecutor.get()) {
+            if (destroyed || executor->commandExecutor) {
               taskState = TASK_FAILED;
             } else {
               taskState = TASK_LOST;
@@ -2176,8 +2178,7 @@ void Slave::executorTerminated(
         // supports it.
         foreach (const TaskInfo& task, executor->queuedTasks.values()) {
           mesos::TaskState taskState;
-          isCommandExecutor = task.has_command();
-          if (destroyed || isCommandExecutor.get()) {
+          if (destroyed || executor->commandExecutor) {
             taskState = TASK_FAILED;
           } else {
             taskState = TASK_LOST;
@@ -2192,16 +2193,10 @@ void Slave::executorTerminated(
         }
       }
 
-      // If we weren't able to figure out whether this executor is a
-      // command executor above (e.g., no pending tasks), we deduce
-      // it from the ExecutorInfo. This is a hack for now.
-      if (isCommandExecutor.isNone()) {
-        isCommandExecutor = strings::contains(
-            executor->info.command().value(),
-            path::join(flags.launcher_dir, "mesos-executor"));
-      }
-
-      if (!isCommandExecutor.get()) {
+      // Only send ExitedExecutorMessage if it is not a Command
+      // Executor because the master doesn't store them; they are
+      // generated by the slave.
+      if (!executor->commandExecutor) {
         ExitedExecutorMessage message;
         message.mutable_slave_id()->MergeFrom(info.id());
         message.mutable_framework_id()->MergeFrom(frameworkId);
@@ -2998,6 +2993,9 @@ Executor::Executor(
     uuid(_uuid),
     directory(_directory),
     checkpoint(_checkpoint),
+    commandExecutor(strings::contains(
+        info.command().value(),
+        path::join(slave->flags.launcher_dir, "mesos-executor"))),
     pid(UPID()),
     resources(_info.resources()),
     completedTasks(MAX_COMPLETED_TASKS_PER_EXECUTOR)

http://git-wip-us.apache.org/repos/asf/mesos/blob/7ca371ce/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 93fd32e..ef8b64f 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -375,6 +375,8 @@ struct Executor
 
   const bool checkpoint;
 
+  const bool commandExecutor;
+
   UPID pid;
 
   Resources resources; // Currently consumed resources.