Repository: mesos Updated Branches: refs/heads/master 3587ca14c -> db8d097c9
Moved task validation from `getExecutorInfo` to `runTask` on agent. Previously, `getExecutorInfo` was called only in `runTask`, so it asserted the invariant that a task should have either CommandInfo or ExecutorInfo set but not both. This is true for individual tasks, but it is not necessarily true for tasks which are part of a task group, since the master injects the task group's ExecutorInfo. Now `getExecutorInfo` is also called to calculate allocated resources of tasks which might be part of a task group, which could violate this invariant, so the assertion has been moved. Review: https://reviews.apache.org/r/61524/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4f480739 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4f480739 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4f480739 Branch: refs/heads/master Commit: 4f4807394944d23d3a6f79249ce49e2494a88350 Parents: 3587ca1 Author: Andrei Budnik <abud...@mesosphere.com> Authored: Wed Aug 9 11:06:40 2017 -0700 Committer: Greg Mann <gregorywm...@gmail.com> Committed: Wed Aug 9 15:37:12 2017 -0700 ---------------------------------------------------------------------- src/slave/slave.cpp | 11 +++++++---- src/slave/slave.hpp | 5 +++-- 2 files changed, 10 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/4f480739/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 7381530..5a45054 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -1597,6 +1597,10 @@ void Slave::runTask( const UPID& pid, const TaskInfo& task) { + CHECK_NE(task.has_executor(), task.has_command()) + << "Task " << task.task_id() + << " should have either CommandInfo or ExecutorInfo set but not both"; + if (master != from) { LOG(WARNING) << "Ignoring run task message from " << from << " because it is not the expected master: " @@ -4980,10 +4984,9 @@ ExecutorInfo Slave::getExecutorInfo( const FrameworkInfo& frameworkInfo, const TaskInfo& task) const { - CHECK_NE(task.has_executor(), task.has_command()) - << "Task " << task.task_id() - << " should have either CommandInfo or ExecutorInfo set but not both"; - + // In the case of tasks launched as part of a task group, the task group's + // ExecutorInfo is injected into each TaskInfo by the master and we return + // it here. if (task.has_executor()) { return task.executor(); } http://git-wip-us.apache.org/repos/asf/mesos/blob/4f480739/src/slave/slave.hpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp index 1fe93da..0e07a1a 100644 --- a/src/slave/slave.hpp +++ b/src/slave/slave.hpp @@ -396,8 +396,9 @@ public: Executor* getExecutor(const ContainerID& containerId) const; - // Returns an ExecutorInfo for a TaskInfo (possibly - // constructing one if the task has a CommandInfo). + // Returns the ExecutorInfo associated with a TaskInfo. If the task has no + // ExecutorInfo, then we generate an ExecutorInfo corresponding to the + // command executor. ExecutorInfo getExecutorInfo( const FrameworkInfo& frameworkInfo, const TaskInfo& task) const;
