You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2018/01/12 22:53:17 UTC
mesos git commit: Prevented a crash when an agent with terminal tasks
is partitioned.
Repository: mesos
Updated Branches:
refs/heads/master bbd461773 -> ecf718795
Prevented a crash when an agent with terminal tasks is partitioned.
If an agent is lost, we try to remove all the tasks that might have
been lost. If a task is already terminal but has unacknowleged status
updates, it is expected that we track it in the unreachable tasks list
so we should remove the CHECK that prevents this. This patch also
changes to how unreachable tasks are presented in the HTTP endpoints
so that terminal but unacknowleged tasks are shown in in the list of
unreachable tasks and not completed tasks, which is different than
1.4.x where they are shown as completed.
Review: https://reviews.apache.org/r/64940/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ecf71879
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ecf71879
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ecf71879
Branch: refs/heads/master
Commit: ecf718795bbbc06473ba43b7dbd9107446e1a15e
Parents: bbd4617
Author: James Peach <jp...@apache.org>
Authored: Fri Jan 12 13:46:27 2018 -0800
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Jan 12 14:52:56 2018 -0800
----------------------------------------------------------------------
src/master/http.cpp | 31 +------
src/master/master.cpp | 2 -
src/master/master.hpp | 2 -
src/tests/mesos.hpp | 15 +++-
src/tests/partition_tests.cpp | 168 +++++++++++++++++++++++++++++++++++--
5 files changed, 173 insertions(+), 45 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecf71879/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index bc29faf..0c2cd55 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -328,12 +328,6 @@ struct FullFrameworkWriter {
writer->field("unreachable_tasks", [this](JSON::ArrayWriter* writer) {
foreachvalue (const Owned<Task>& task, framework_->unreachableTasks) {
- // There could be TASK_LOST tasks in this map. See comment for
- // `unreachableTasks`.
- if (task->state() != TASK_UNREACHABLE) {
- continue;
- }
-
// Skip unauthorized tasks.
if (!authorizeTask_->accept(*task, framework_->info)) {
continue;
@@ -352,22 +346,6 @@ struct FullFrameworkWriter {
writer->element(*task);
}
-
- // Unreachable tasks belonging to a non-partition-aware framework
- // have been stored as TASK_LOST for backward compatibility so we
- // should export them as completed tasks.
- foreachvalue (const Owned<Task>& task, framework_->unreachableTasks) {
- if (task->state() != TASK_LOST) {
- continue;
- }
-
- // Skip unauthorized tasks.
- if (!authorizeTask_->accept(*task, framework_->info)) {
- continue;
- }
-
- writer->element(*task);
- }
});
// Model all of the offers associated with a framework.
@@ -4251,14 +4229,7 @@ mesos::master::Response::GetTasks Master::Http::_getTasks(
continue;
}
- if (task->state() == TASK_UNREACHABLE) {
- getTasks.add_unreachable_tasks()->CopyFrom(*task);
- } else {
- // Unreachable tasks belonging to a non-partition-aware framework
- // have been stored as TASK_LOST for backward compatibility so we
- // should export them as completed tasks.
- getTasks.add_completed_tasks()->CopyFrom(*task);
- }
+ getTasks.add_unreachable_tasks()->CopyFrom(*task);
}
// Completed tasks.
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecf71879/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index c96cd70..8738e10 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -10208,8 +10208,6 @@ void Master::removeTask(Task* task, bool unreachable)
const Resources resources = task->resources();
if (!isRemovable(task->state())) {
- CHECK(!unreachable) << task->task_id();
-
// Note that we use `Resources` for output as it's faster than
// logging raw protobuf data.
LOG(WARNING) << "Removing task " << task->task_id()
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecf71879/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index f915c6f..0c05b20 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -2328,8 +2328,6 @@ struct Framework
}
if (unreachable) {
- CHECK(task->state() == TASK_UNREACHABLE || task->state() == TASK_LOST)
- << task->state();
addUnreachableTask(*task);
} else {
addCompletedTask(Task(*task));
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecf71879/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index 93913f2..16c75bb 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -3494,21 +3494,30 @@ MATCHER_P(OffersHaveResource, resource, "")
}
-// This matcher is used to match the task id of `TaskStatus` message.
+// This matcher is used to match the task id of a `TaskStatus` message.
MATCHER_P(TaskStatusTaskIdEq, taskInfo, "")
{
return arg.task_id() == taskInfo.task_id();
}
-// This matcher is used to match the task id of `Event.update.status` message.
+// This matcher is used to match the state of a `TaskStatus` message.
+MATCHER_P(TaskStatusStateEq, taskState, "")
+{
+ return arg.state() == taskState;
+}
+
+
+// This matcher is used to match the task id of an `Event.update.status`
+// message.
MATCHER_P(TaskStatusUpdateTaskIdEq, taskInfo, "")
{
return arg.status().task_id() == taskInfo.task_id();
}
-// This matcher is used to match the state of `Event.update.status` message.
+// This matcher is used to match the state of an `Event.update.status`
+// message.
MATCHER_P(TaskStatusUpdateStateEq, taskState, "")
{
return arg.status().state() == taskState;
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecf71879/src/tests/partition_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/partition_tests.cpp b/src/tests/partition_tests.cpp
index 3813139..a9036a1 100644
--- a/src/tests/partition_tests.cpp
+++ b/src/tests/partition_tests.cpp
@@ -69,6 +69,7 @@ using process::http::Response;
using std::vector;
using testing::_;
+using testing::AllOf;
using testing::AtMost;
using testing::DoAll;
using testing::Eq;
@@ -678,22 +679,30 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, ReregisterSlaveNotPartitionAware)
EXPECT_TRUE(runningTasks.values.empty());
+ // Although the task state based metrics above show the task in
+ // master/tasks_lost, the state endpoint lists the task in the
+ // "unreachable_tasks" section because it is indeed unreachable
+ // and shouldn't be considered "completed". This difference is
+ // unfortunate and should be addressed in MESOS-8405.
JSON::Array unreachableTasks =
framework.values["unreachable_tasks"].as<JSON::Array>();
- EXPECT_TRUE(unreachableTasks.values.empty());
+ EXPECT_FALSE(unreachableTasks.values.empty());
- JSON::Array completedTasks =
- framework.values["completed_tasks"].as<JSON::Array>();
-
- JSON::Object completedTask =
- completedTasks.values.front().as<JSON::Object>();
+ JSON::Object unreachableTask =
+ unreachableTasks.values.front().as<JSON::Object>();
+ // The unreachable task is in its backwards-compatible state.
EXPECT_EQ(
- task.task_id(), completedTask.values["id"].as<JSON::String>().value);
+ task.task_id(), unreachableTask.values["id"].as<JSON::String>().value);
EXPECT_EQ(
"TASK_LOST",
- completedTask.values["state"].as<JSON::String>().value);
+ unreachableTask.values["state"].as<JSON::String>().value);
+
+ JSON::Array completedTasks =
+ framework.values["completed_tasks"].as<JSON::Array>();
+
+ EXPECT_TRUE(completedTasks.values.empty());
}
// We now complete the partition on the slave side as well. We
@@ -2395,6 +2404,149 @@ TEST_F(PartitionTest, PartitionAwareTaskCompletedOnPartitionedAgent)
}
+// This test verifies that when the master removes a lost agent any
+// unacknowledged but terminal tasks on the agent are tracked as
+// unreachable but keep their original terminal state.
+TEST_F(PartitionTest, AgentWithTerminalTaskPartitioned)
+{
+ // Start a master.
+ master::Flags masterFlags = CreateMasterFlags();
+ Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
+ ASSERT_SOME(master);
+
+ StandaloneMasterDetector detector(master.get()->pid);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(&detector);
+ ASSERT_SOME(slave);
+
+ // We require the completed task to still be tracked on the master when
+ // we lose the agent. By disabling scheduler implicit acknowledgements
+ // and controlling the acknowledgements manually, we ensure that the
+ // master is forced to retain the task state.
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched,
+ DEFAULT_FRAMEWORK_INFO,
+ master.get()->pid,
+ false,
+ DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillRepeatedly(Return());
+
+ Future<vector<Offer>> offers;
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ driver.start();
+
+ AWAIT_READY(offers);
+
+ testing::Sequence taskSequence;
+ Future<TaskStatus> starting;
+ Future<TaskStatus> running;
+ Future<TaskStatus> finished;
+
+ TaskInfo task = createTask(offers->at(0), SLEEP_COMMAND(0));
+
+ EXPECT_CALL(
+ sched,
+ statusUpdate(&driver, AllOf(
+ TaskStatusTaskIdEq(task),
+ TaskStatusStateEq(TASK_STARTING))))
+ .InSequence(taskSequence)
+ .WillOnce(FutureArg<1>(&starting));
+
+ EXPECT_CALL(
+ sched,
+ statusUpdate(&driver, AllOf(
+ TaskStatusTaskIdEq(task),
+ TaskStatusStateEq(TASK_RUNNING))))
+ .InSequence(taskSequence)
+ .WillOnce(FutureArg<1>(&running));
+
+ EXPECT_CALL(
+ sched,
+ statusUpdate(&driver, AllOf(
+ TaskStatusTaskIdEq(task),
+ TaskStatusStateEq(TASK_FINISHED))))
+ .InSequence(taskSequence)
+ .WillOnce(FutureArg<1>(&finished))
+ // Ignore additional status update delivery attempts.
+ .WillRepeatedly(Return());
+
+ Clock::pause();
+
+ driver.launchTasks(offers->at(0).id(), {task});
+
+ AWAIT_READY(starting);
+ driver.acknowledgeStatusUpdate(starting.get());
+
+ AWAIT_READY(running);
+ driver.acknowledgeStatusUpdate(running.get());
+
+ // Wait for the task to finish but don't acknowledge the status
+ // update. This ensures that the master is still tracking the task
+ // when the agent becomes lost.
+ AWAIT_READY(finished);
+
+ // When the agent is lost, the master will rescind any offers and we
+ // can just ignore that.
+ EXPECT_CALL(sched, offerRescinded(&driver, _))
+ .WillRepeatedly(Return());
+
+ EXPECT_CALL(sched, slaveLost(&driver, _))
+ .Times(1);
+
+ Future<Nothing> unreachable =
+ FUTURE_DISPATCH(_, &master::Master::markUnreachable);
+
+ DROP_PROTOBUFS(PongSlaveMessage(), _, _);
+
+ // Now fast forward through the ping timeouts so that the agent
+ // becomes unreachable on the master.
+ for (unsigned i = 0; i <= masterFlags.max_agent_ping_timeouts; ++i) {
+ Future<PingSlaveMessage> ping = FUTURE_PROTOBUF(PingSlaveMessage(), _, _);
+ Clock::advance(masterFlags.agent_ping_timeout);
+ AWAIT_READY(ping);
+ }
+
+ AWAIT_READY(unreachable);
+
+ Clock::settle();
+ Clock::resume();
+
+ Future<Response> response = process::http::get(
+ master.get()->pid,
+ "frameworks",
+ None(),
+ createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+
+ Try<JSON::Object> json = JSON::parse<JSON::Object>(response->body);
+ ASSERT_SOME(json);
+
+ // We should not have any active tasks.
+ EXPECT_NONE(json->find<JSON::Number>("frameworks[0].tasks[0]"));
+
+ // The task we launched should be unreachable (since the final status
+ // update wasn't acknowledged) but still terminal. Note that these
+ // expectations are likely to change as part of MESOS-8405.
+ EXPECT_SOME_EQ(
+ task.task_id().value(),
+ json->find<JSON::String>("frameworks[0].unreachable_tasks[0].id"));
+ EXPECT_SOME_EQ(
+ "TASK_FINISHED",
+ json->find<JSON::String>("frameworks[0].unreachable_tasks[0].state"));
+
+ driver.stop();
+ driver.join();
+}
+
+
// This test checks that the master correctly garbage collects
// information about unreachable agents from the registry using the
// count-based GC criterion.