You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2017/11/08 16:55:38 UTC

[2/3] mesos git commit: Fixed incorrect validation that forced ExecutorID uniqueness.

Fixed incorrect validation that forced ExecutorID uniqueness.

In the ReregisterSlaveMessage message validation, we were
requiring the ExecutorID to be unique across a whole agent.
In fact, only the {FrameworkID, ExecutorID} tuple must be
unique.

This closes #248


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

Branch: refs/heads/1.4.x
Commit: ed1f00e356a782892d20b31905fe45f775082710
Parents: a2b346b
Author: James DeFelice <ja...@gmail.com>
Authored: Mon Nov 6 16:53:41 2017 -0800
Committer: James Peach <jp...@apache.org>
Committed: Wed Nov 8 08:25:21 2017 -0800

----------------------------------------------------------------------
 src/master/validation.cpp | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/ed1f00e3/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index 279dd51..d029520 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -44,6 +44,7 @@
 
 using process::http::authentication::Principal;
 
+using std::pair;
 using std::set;
 using std::string;
 using std::vector;
@@ -284,7 +285,7 @@ Option<Error> reregisterSlave(
     const vector<FrameworkInfo>& frameworkInfos)
 {
   hashset<FrameworkID> frameworkIDs;
-  hashset<ExecutorID> executorIDs;
+  hashset<pair<FrameworkID, ExecutorID>> executorIDs;
 
   Option<Error> error = validateSlaveInfo(slaveInfo);
   if (error.isSome()) {
@@ -336,12 +337,14 @@ Option<Error> reregisterSlave(
     }
 
     if (executor.has_executor_id()) {
-      if (executorIDs.contains(executor.executor_id())) {
-        return Error("Executor has a duplicate ExecutorID '" +
+      auto id = std::make_pair(executor.framework_id(), executor.executor_id());
+      if (executorIDs.contains(id)) {
+        return Error("Framework '" + stringify(executor.framework_id()) +
+                     "' has a duplicate ExecutorID '" +
                      stringify(executor.executor_id()) + "'");
       }
 
-      executorIDs.insert(executor.executor_id());
+      executorIDs.insert(id);
     }
   }
 
@@ -365,7 +368,8 @@ Option<Error> reregisterSlave(
     // is generated on the agent (see Slave::doReliableRegistration). Only
     // running tasks ought to have executors.
     if (task.has_executor_id() && task.state() == TASK_RUNNING) {
-      if (!executorIDs.contains(task.executor_id())) {
+      auto id = std::make_pair(task.framework_id(), task.executor_id());
+      if (!executorIDs.contains(id)) {
         return Error("Task has an invalid ExecutorID '" +
                      stringify(task.executor_id()) + "'");
       }