Updated master and slave to properly set task status uuid. This just makes sure master and slave properly set the uuid in task status to setup the stage for deprecating the messy logic in scheduler driver in a future release.
Review: https://reviews.apache.org/r/39792 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/48a96acd Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/48a96acd Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/48a96acd Branch: refs/heads/master Commit: 48a96acd9e801803ee4bb5acd82444e32cab4fea Parents: 2dd7cf8 Author: Vinod Kone <[email protected]> Authored: Fri Nov 6 09:27:50 2015 -0800 Committer: Vinod Kone <[email protected]> Committed: Fri Nov 6 10:09:58 2015 -0800 ---------------------------------------------------------------------- src/master/master.cpp | 13 ++++++++++++- src/master/master.hpp | 2 +- src/sched/sched.cpp | 5 +++-- src/slave/slave.cpp | 12 +++++++++--- 4 files changed, 25 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/48a96acd/src/master/master.cpp ---------------------------------------------------------------------- diff --git a/src/master/master.cpp b/src/master/master.cpp index cbae27e..8107960 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -4368,7 +4368,9 @@ void Master::updateUnavailability( // TODO(vinod): Since 0.22.0, we can use 'from' instead of 'pid' // because the status updates will be sent by the slave. -void Master::statusUpdate(const StatusUpdate& update, const UPID& pid) +// +// TODO(vinod): Add a benchmark test for status update handling. +void Master::statusUpdate(StatusUpdate update, const UPID& pid) { ++metrics->messages_status_update; @@ -4411,6 +4413,15 @@ void Master::statusUpdate(const StatusUpdate& update, const UPID& pid) LOG(INFO) << "Status update " << update << " from slave " << *slave; + // We ensure that the uuid of task status matches the update's uuid, in case + // the task status uuid is not set by the slave. + // + // TODO(vinod): This can be `CHECK(update.status().has_uuid())` from 0.27.0 + // since a >= 0.26.0 slave will always correctly set task status uuid. + if (update.has_uuid()) { + update.mutable_status()->set_uuid(update.uuid()); + } + // Forward the update to the framework. forward(update, pid, framework); http://git-wip-us.apache.org/repos/asf/mesos/blob/48a96acd/src/master/master.hpp ---------------------------------------------------------------------- diff --git a/src/master/master.hpp b/src/master/master.hpp index b19f23b..d692185 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -468,7 +468,7 @@ public: const SlaveID& slaveId); void statusUpdate( - const StatusUpdate& update, + StatusUpdate update, const process::UPID& pid); void reconcileTasks( http://git-wip-us.apache.org/repos/asf/mesos/blob/48a96acd/src/sched/sched.cpp ---------------------------------------------------------------------- diff --git a/src/sched/sched.cpp b/src/sched/sched.cpp index 4ec1633..a6faf92 100644 --- a/src/sched/sched.cpp +++ b/src/sched/sched.cpp @@ -900,8 +900,9 @@ protected: // update uuid check here, until then we must still check for // this being sent from the driver (from == UPID()) or from // the master (pid == UPID()). - // TODO(vinod): Get rid of this logic in 0.25.0 because master - // and slave correctly set task status in 0.24.0. + // + // TODO(vinod): Get rid of this logic in 0.27.0 because master + // correctly sets task status since 0.26.0. if (!update.has_uuid() || update.uuid() == "") { status.clear_uuid(); } else if (from == UPID() || pid == UPID()) { http://git-wip-us.apache.org/repos/asf/mesos/blob/48a96acd/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index a0efba7..7d70e86 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -3028,6 +3028,15 @@ void Slave::forward(StatusUpdate update) return; } + // Ensure that task status uuid is set even if this update was sent by the + // status update manager after recovering a pre 0.23.x slave/executor driver's + // updates. This allows us to simplify the master code (in >= 0.27.0) to asume + // the uuid is always set for retryable updates. + CHECK(update.has_uuid()) + << "Expecting updates without 'uuid' to have been rejected"; + + update.mutable_status()->set_uuid(update.uuid()); + // Update the status update state of the task and include the latest // state of the task in the status update. Framework* framework = getFramework(update.framework_id()); @@ -3047,9 +3056,6 @@ void Slave::forward(StatusUpdate update) } if (task != NULL) { - CHECK(update.has_uuid()) - << "Expecting updates without 'uuid' to have been rejected"; - // We set the status update state of the task here because in // steady state master updates the status update state of the // task when it receives this update. If the master fails over,
