[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mesos-commits
Subject:    [2/3] git commit: Updated the slave to handle status update acknowledgements from the master.
From:       bmahler () apache ! org
Date:       2014-05-27 22:41:14
Message-ID: c0193478b6984331b0335ef00784f170 () git ! apache ! org
[Download RAW message or body]

Updated the slave to handle status update acknowledgements from the master.

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


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

Branch: refs/heads/master
Commit: 2986ac8be22fe1c295d8eeced714fef3344815f7
Parents: 3dd88aa
Author: Benjamin Mahler <bmahler@twitter.com>
Authored: Thu May 22 11:40:16 2014 -0700
Committer: Benjamin Mahler <bmahler@twitter.com>
Committed: Tue May 27 15:41:03 2014 -0700

----------------------------------------------------------------------
 src/slave/slave.cpp                       | 31 ++++++++++++++++++++++++++
 src/slave/slave.hpp                       |  1 +
 src/tests/status_update_manager_tests.cpp |  2 ++
 3 files changed, 34 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2986ac8b/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index e8fc7c6..7fbedb1 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -1540,11 +1540,42 @@ void Slave::updateFramework(const FrameworkID& frameworkId, const string& pid)
 
 
 void Slave::statusUpdateAcknowledgement(
+    const UPID& from,
     const SlaveID& slaveId,
     const FrameworkID& frameworkId,
     const TaskID& taskId,
     const string& uuid)
 {
+  // Originally, all status update acknowledgements were sent from the
+  // scheduler driver. We'd like to have all acknowledgements sent by
+  // the master instead. See: MESOS-1389.
+  // For now, we handle acknowledgements from the leading master and
+  // from the scheduler driver, for backwards compatibility.
+  // TODO(bmahler): Aim to have the scheduler driver no longer
+  // sending acknowledgements in 0.20.0. Stop handling those messages
+  // here in 0.21.0.
+  // NOTE: We must reject those acknowledgements coming from
+  // non-leading masters because we may have already sent the terminal
+  // un-acknowledged task to the leading master! Unfortunately, the
+  // master's pid will not change across runs on the same machine, so
+  // we may process a message from the old master on the same machine,
+  // but this is a more general problem!
+  if (strings::startsWith(from.id, "master")) {
+    if (state != RUNNING) {
+      LOG(WARNING) << "Dropping status update acknowledgement message for "
+                   << frameworkId << " because the slave is in "
+                   << state << " state";
+      return;
+    }
+
+    if (master != from) {
+      LOG(WARNING) << "Ignoring status update acknowledgement message from "
+                   << from << " because it is not the expected master: "
+                   << (master.isSome() ? stringify(master.get()) : "None");
+      return;
+    }
+  }
+
   statusUpdateManager->acknowledgement(
       taskId, frameworkId, UUID::fromBytes(uuid))
     .onAny(defer(self(),

http://git-wip-us.apache.org/repos/asf/mesos/blob/2986ac8b/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 57f84f7..34687e5 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -173,6 +173,7 @@ public:
       const process::UPID& pid);
 
   void statusUpdateAcknowledgement(
+      const process::UPID& from,
       const SlaveID& slaveId,
       const FrameworkID& frameworkId,
       const TaskID& taskId,

http://git-wip-us.apache.org/repos/asf/mesos/blob/2986ac8b/src/tests/status_update_manager_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/status_update_manager_tests.cpp b/src/tests/status_update_manager_tests.cpp
index 8cbfb83..c753921 100644
--- a/src/tests/status_update_manager_tests.cpp
+++ b/src/tests/status_update_manager_tests.cpp
@@ -355,6 +355,7 @@ TEST_F(StatusUpdateManagerTest, IgnoreDuplicateStatusUpdateAck)
   process::dispatch(
       slave.get(),
       &Slave::statusUpdateAcknowledgement,
+      master.get(),
       update.slave_id(),
       frameworkId,
       update.status().task_id(),
@@ -447,6 +448,7 @@ TEST_F(StatusUpdateManagerTest, IgnoreUnexpectedStatusUpdateAck)
   process::dispatch(
       slave.get(),
       &Slave::statusUpdateAcknowledgement,
+      master.get(),
       update.slave_id(),
       frameworkId,
       update.status().task_id(),


[prev in list] [next in list] [prev in thread] [next in thread]