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 2015/11/06 19:10:09 UTC
[1/5] mesos git commit: Made slave version required in master.cpp.
Repository: mesos
Updated Branches:
refs/heads/master 39b949b31 -> 6d28a8474
Made slave version required in master.cpp.
Slave version is being set by the slave since 0.21.0. Cleaned up the
code in master that expected it to be optional.
Review: https://reviews.apache.org/r/39827
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c8277ef2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c8277ef2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c8277ef2
Branch: refs/heads/master
Commit: c8277ef2a7a4ec88b1d09d8cdc35d1cd5e1e0e20
Parents: 48a96ac
Author: Vinod Kone <vi...@gmail.com>
Authored: Fri Nov 6 09:28:42 2015 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Nov 6 10:09:58 2015 -0800
----------------------------------------------------------------------
src/master/http.cpp | 4 +--
src/master/master.cpp | 64 +++++++++++++++-------------------------------
src/master/master.hpp | 6 ++---
3 files changed, 23 insertions(+), 51 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/c8277ef2/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 9d20346..3b2cded 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -295,10 +295,8 @@ JSON::Object summarize(const Slave& slave)
object.values["attributes"] = model(slave.info.attributes());
object.values["active"] = slave.active;
+ object.values["version"] = slave.version;
- if (slave.version.isSome()) {
- object.values["version"] = slave.version.get();
- }
return object;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/c8277ef2/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 8107960..bad38ff 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -3906,7 +3906,7 @@ void Master::_registerSlave(
slaveInfo,
pid,
machineId,
- version.empty() ? Option<string>::none() : version,
+ version,
Clock::now(),
checkpointedResources);
@@ -4147,7 +4147,7 @@ void Master::_reregisterSlave(
slaveInfo,
pid,
machineId,
- version.empty() ? Option<string>::none() : version,
+ version,
Clock::now(),
checkpointedResources,
executorInfos,
@@ -5243,51 +5243,27 @@ void Master::reconcile(
LOG(WARNING) << "Task " << task->task_id()
<< " of framework " << task->framework_id()
<< " unknown to the slave " << *slave
- << " during re-registration"
- << (slave->version.isSome()
- ? ": reconciling with the slave"
- : ": sending TASK_LOST");
-
- if (slave->version.isSome()) {
- // NOTE: Currently the slave doesn't look at the task state
- // when it reconciles the task state; we include the correct
- // state for correctness and consistency.
- const TaskState& state = task->has_status_update_state()
- ? task->status_update_state()
- : task->state();
-
- TaskStatus* status = reconcile.add_statuses();
- status->mutable_task_id()->CopyFrom(task->task_id());
- status->mutable_slave_id()->CopyFrom(slave->id);
- status->set_state(state);
- status->set_source(TaskStatus::SOURCE_MASTER);
- status->set_message("Reconciliation request");
- status->set_reason(TaskStatus::REASON_RECONCILIATION);
- status->set_timestamp(Clock::now().secs());
- } else {
- // TODO(bmahler): Remove this case in 0.22.0.
- const StatusUpdate& update = protobuf::createStatusUpdate(
- task->framework_id(),
- slave->id,
- task->task_id(),
- TASK_LOST,
- TaskStatus::SOURCE_MASTER,
- None(),
- "Task is unknown to the slave",
- TaskStatus::REASON_TASK_UNKNOWN);
-
- updateTask(task, update);
- removeTask(task);
-
- Framework* framework = getFramework(frameworkId);
- if (framework != NULL) {
- forward(update, UPID(), framework);
- }
- }
+ << " during re-registration : reconciling with the slave";
+
+ // NOTE: Currently the slave doesn't look at the task state
+ // when it reconciles the task state; we include the correct
+ // state for correctness and consistency.
+ const TaskState& state = task->has_status_update_state()
+ ? task->status_update_state()
+ : task->state();
+
+ TaskStatus* status = reconcile.add_statuses();
+ status->mutable_task_id()->CopyFrom(task->task_id());
+ status->mutable_slave_id()->CopyFrom(slave->id);
+ status->set_state(state);
+ status->set_source(TaskStatus::SOURCE_MASTER);
+ status->set_message("Reconciliation request");
+ status->set_reason(TaskStatus::REASON_RECONCILIATION);
+ status->set_timestamp(Clock::now().secs());
}
}
- if (slave->version.isSome() && reconcile.statuses_size() > 0) {
+ if (reconcile.statuses_size() > 0) {
reregistered.add_reconciliations()->CopyFrom(reconcile);
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/c8277ef2/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index d692185..ead8520 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -111,7 +111,7 @@ struct Slave
Slave(const SlaveInfo& _info,
const process::UPID& _pid,
const MachineID& _machineId,
- const Option<std::string> _version,
+ const std::string& _version,
const process::Time& _registeredTime,
const Resources& _checkpointedResources,
const std::vector<ExecutorInfo> executorInfos =
@@ -303,10 +303,8 @@ struct Slave
process::UPID pid;
- // The Mesos version of the slave. If set, the slave is >= 0.21.0.
// TODO(bmahler): Use stout's Version when it can parse labels, etc.
- // TODO(bmahler): Make this required once it is always set.
- const Option<std::string> version;
+ const std::string version;
process::Time registeredTime;
Option<process::Time> reregisteredTime;
[2/5] mesos git commit: Updated master and slave to properly set task
status uuid.
Posted by vi...@apache.org.
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 <vi...@gmail.com>
Authored: Fri Nov 6 09:27:50 2015 -0800
Committer: Vinod Kone <vi...@gmail.com>
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,
[4/5] mesos git commit: Updated code and comments in
Master::updateTask() that deals with 0.21.0 slaves.
Posted by vi...@apache.org.
Updated code and comments in Master::updateTask() that deals with
0.21.0 slaves.
The only functional change here is that updateTask() doesn't check for
uuid being set to empty string.
Review: https://reviews.apache.org/r/39878
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6aa7004c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6aa7004c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6aa7004c
Branch: refs/heads/master
Commit: 6aa7004c153c8106589da70c17a35b7012a64711
Parents: c8277ef
Author: Vinod Kone <vi...@gmail.com>
Authored: Fri Nov 6 09:29:35 2015 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Nov 6 10:09:59 2015 -0800
----------------------------------------------------------------------
src/master/master.cpp | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6aa7004c/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index bad38ff..70e55a5 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -6014,7 +6014,7 @@ void Master::updateTask(Task* task, const StatusUpdate& update)
return;
}
- // Get the latest state.
+ // Updates from the slave have 'latest_state' set.
Option<TaskState> latestState;
if (update.has_latest_state()) {
latestState = update.latest_state();
@@ -6024,14 +6024,11 @@ void Master::updateTask(Task* task, const StatusUpdate& update)
// transitioned to terminal state. Also set the latest state.
bool terminated;
if (latestState.isSome()) {
- // This update must be from >= 0.21.0 slave.
terminated = !protobuf::isTerminalState(task->state()) &&
protobuf::isTerminalState(latestState.get());
task->set_state(latestState.get());
} else {
- // This update must be from a pre 0.21.0 slave or generated by the
- // master.
terminated = !protobuf::isTerminalState(task->state()) &&
protobuf::isTerminalState(status.state());
@@ -6041,7 +6038,7 @@ void Master::updateTask(Task* task, const StatusUpdate& update)
// Set the status update state and uuid for the task. Note that
// master-generated updates are terminal and do not have a uuid
// (in which case the master also calls removeTask()).
- if (update.has_uuid() && update.uuid() != "") {
+ if (update.has_uuid()) {
task->set_status_update_state(status.state());
task->set_status_update_uuid(update.uuid());
}
@@ -6061,12 +6058,10 @@ void Master::updateTask(Task* task, const StatusUpdate& update)
// MESOS-1746.
task->mutable_statuses(task->statuses_size() - 1)->clear_data();
- LOG(INFO) << "Updating the latest state of task " << task->task_id()
+ LOG(INFO) << "Updating the state of task " << task->task_id()
<< " of framework " << task->framework_id()
- << " to " << task->state()
- << (task->state() != status.state()
- ? " (status update state: " + stringify(status.state()) + ")"
- : "");
+ << " (latest state: " << task->state()
+ << ", status update state: " << status.state() << ")";
// Once the task becomes terminal, we recover the resources.
if (terminated) {
[3/5] mesos git commit: Updated createStatusUpdate() to unset
StatusUpdate.uuid instead of setting it to an empty string.
Posted by vi...@apache.org.
Updated createStatusUpdate() to unset StatusUpdate.uuid instead of
setting it to an empty string.
As the deleted comment says, we were setting it to emtpy because the
'uuid' was a required field until 0.22.x. Since we are at 0.26 now,
it should be safe to just unset it.
Review: https://reviews.apache.org/r/39791
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2dd7cf8f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2dd7cf8f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2dd7cf8f
Branch: refs/heads/master
Commit: 2dd7cf8f469783090f067b818d7ddf8f39eccf9d
Parents: 39b949b
Author: Vinod Kone <vi...@gmail.com>
Authored: Fri Nov 6 09:27:04 2015 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Nov 6 10:09:58 2015 -0800
----------------------------------------------------------------------
src/common/protobuf_utils.cpp | 9 ---------
src/messages/messages.proto | 8 +++++---
2 files changed, 5 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2dd7cf8f/src/common/protobuf_utils.cpp
----------------------------------------------------------------------
diff --git a/src/common/protobuf_utils.cpp b/src/common/protobuf_utils.cpp
index 1e795dc..b3c2d11 100644
--- a/src/common/protobuf_utils.cpp
+++ b/src/common/protobuf_utils.cpp
@@ -95,15 +95,6 @@ StatusUpdate createStatusUpdate(
if (uuid.isSome()) {
update.set_uuid(uuid.get().toBytes());
status->set_uuid(uuid.get().toBytes());
- } else {
- // Note that in 0.22.x, the StatusUpdate.uuid was required
- // even though the scheduler driver ignores it for master
- // and scheduler driver generated updates. So we continue
- // to "set" it here so that updates coming from a 0.23.x
- // master can be parsed by a 0.22.x scheduler driver.
- //
- // TODO(bmahler): In 0.24.x, leave the uuid unset.
- update.set_uuid("");
}
if (reason.isSome()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/2dd7cf8f/src/messages/messages.proto
----------------------------------------------------------------------
diff --git a/src/messages/messages.proto b/src/messages/messages.proto
index 9a1564f..5714087 100644
--- a/src/messages/messages.proto
+++ b/src/messages/messages.proto
@@ -82,12 +82,14 @@ message StatusUpdate {
required FrameworkID framework_id = 1;
optional ExecutorID executor_id = 2;
optional SlaveID slave_id = 3;
+
+ // Since 0.23.0 we set 'status.uuid' in the executor
+ // driver for all retryable status updates.
required TaskStatus status = 4;
+
required double timestamp = 5;
- // This is being deprecated in favor of TaskStatus.uuid. In 0.23.0,
- // we set the TaskStatus 'uuid' in the executor driver for all
- // retryable status updates.
+ // Since 0.26.0 this is deprecated in favor of 'status.uuid'.
optional bytes uuid = 6;
// This corresponds to the latest state of the task according to the
[5/5] mesos git commit: Fixed master to properly handle status
updates when multiple of them are enqueued on the slave.
Posted by vi...@apache.org.
Fixed master to properly handle status updates when multiple of them
are enqueued on the slave.
The master now doesn't change the latest state of a task if it has
already terminated. But it still updates the status update state and
uuid.
Review: https://reviews.apache.org/r/39873
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6d28a847
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6d28a847
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6d28a847
Branch: refs/heads/master
Commit: 6d28a84748a3853808997faf42af67310fd75136
Parents: 6aa7004
Author: Vinod Kone <vi...@gmail.com>
Authored: Fri Nov 6 09:30:33 2015 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Nov 6 10:09:59 2015 -0800
----------------------------------------------------------------------
src/master/master.cpp | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6d28a847/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 70e55a5..25b94c8 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -6003,16 +6003,8 @@ void Master::updateTask(Task* task, const StatusUpdate& update)
// Get the unacknowledged status.
const TaskStatus& status = update.status();
- // Once a task's state has been transitioned to terminal state, no further
- // terminal updates should result in a state change. These are the same
- // semantics that are enforced by the slave.
- if (protobuf::isTerminalState(task->state())) {
- LOG(ERROR) << "Ignoring status update for the terminated task "
- << task->task_id()
- << " (" << task->state() << " -> " << status.state() << ")"
- << " of framework " << task->framework_id();
- return;
- }
+ // NOTE: Refer to comments on `StatusUpdate` message in messages.proto for
+ // the difference between `update.latest_state()` and `status.state()`.
// Updates from the slave have 'latest_state' set.
Option<TaskState> latestState;
@@ -6027,17 +6019,26 @@ void Master::updateTask(Task* task, const StatusUpdate& update)
terminated = !protobuf::isTerminalState(task->state()) &&
protobuf::isTerminalState(latestState.get());
- task->set_state(latestState.get());
+ // If the task has already transitioned to a terminal state,
+ // do not update its state.
+ if (!protobuf::isTerminalState(task->state())) {
+ task->set_state(latestState.get());
+ }
} else {
terminated = !protobuf::isTerminalState(task->state()) &&
protobuf::isTerminalState(status.state());
- task->set_state(status.state());
+ // If the task has already transitioned to a terminal state, do not update
+ // its state. Note that we are being defensive here because this should not
+ // happen unless there is a bug in the master code.
+ if (!protobuf::isTerminalState(task->state())) {
+ task->set_state(status.state());
+ }
}
// Set the status update state and uuid for the task. Note that
// master-generated updates are terminal and do not have a uuid
- // (in which case the master also calls removeTask()).
+ // (in which case the master also calls `removeTask()`).
if (update.has_uuid()) {
task->set_status_update_state(status.state());
task->set_status_update_uuid(update.uuid());