You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2015/05/14 01:59:25 UTC
[1/4] mesos git commit: Added tests for slave removal metrics.
Repository: mesos
Updated Branches:
refs/heads/master 288e58807 -> a00ffc855
Added tests for slave removal metrics.
Review: https://reviews.apache.org/r/33155
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a00ffc85
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a00ffc85
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a00ffc85
Branch: refs/heads/master
Commit: a00ffc8556ca472ddf829540b0d39344c8a71b12
Parents: cc39550
Author: Benjamin Mahler <be...@gmail.com>
Authored: Mon Apr 13 17:41:48 2015 -0700
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Wed May 13 16:45:30 2015 -0700
----------------------------------------------------------------------
src/tests/master_tests.cpp | 5 ++++-
src/tests/partition_tests.cpp | 6 ++++++
src/tests/slave_tests.cpp | 4 ++++
3 files changed, 14 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a00ffc85/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 75ffada..ba3858f 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -1375,7 +1375,6 @@ TEST_F(MasterTest, LaunchAcrossSlavesTest)
// Check metrics.
JSON::Object stats = Metrics();
- std::cout << stats << '\n';
EXPECT_EQ(1u, stats.values.count("master/tasks_lost"));
EXPECT_EQ(1u, stats.values["master/tasks_lost"]);
EXPECT_EQ(
@@ -1721,6 +1720,10 @@ TEST_F(MasterTest, RecoveredSlaveDoesNotReregister)
AWAIT_READY(slaveLost);
+ JSON::Object stats = Metrics();
+ EXPECT_EQ(1, stats.values["master/slave_removals"]);
+ EXPECT_EQ(1, stats.values["master/slave_removals/reason_unhealthy"]);
+
Clock::resume();
// Step 7: Ensure the slave cannot re-register!
http://git-wip-us.apache.org/repos/asf/mesos/blob/a00ffc85/src/tests/partition_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/partition_tests.cpp b/src/tests/partition_tests.cpp
index 5e31cac..f7ee3ab 100644
--- a/src/tests/partition_tests.cpp
+++ b/src/tests/partition_tests.cpp
@@ -124,6 +124,12 @@ TEST_F(PartitionTest, PartitionedSlave)
AWAIT_READY(slaveLost);
+ this->Stop(slave.get());
+
+ JSON::Object stats = Metrics();
+ EXPECT_EQ(1, stats.values["master/slave_removals"]);
+ EXPECT_EQ(1, stats.values["master/slave_removals/reason_unhealthy"]);
+
driver.stop();
driver.join();
http://git-wip-us.apache.org/repos/asf/mesos/blob/a00ffc85/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 066cbb0..5bd722e 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -139,6 +139,10 @@ TEST_F(SlaveTest, Shutdown)
AWAIT_READY(offerRescinded);
AWAIT_READY(slaveLost);
+ JSON::Object stats = Metrics();
+ EXPECT_EQ(1, stats.values["master/slave_removals"]);
+ EXPECT_EQ(1, stats.values["master/slave_removals/reason_unregistered"]);
+
driver.stop();
driver.join();
}
[3/4] mesos git commit: Added reason metrics for slave removals.
Posted by bm...@apache.org.
Added reason metrics for slave removals.
Review: https://reviews.apache.org/r/33154
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cc395509
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cc395509
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cc395509
Branch: refs/heads/master
Commit: cc395509a80facbdb22102920430b7f0e04bbc03
Parents: 23a51be
Author: Benjamin Mahler <be...@gmail.com>
Authored: Mon Apr 13 17:19:16 2015 -0700
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Wed May 13 16:45:30 2015 -0700
----------------------------------------------------------------------
include/mesos/mesos.proto | 7 +++++--
src/master/master.cpp | 41 ++++++++++++++++++++++++++++++-----------
src/master/master.hpp | 14 ++++++++++++--
src/master/metrics.cpp | 12 ++++++++++++
src/master/metrics.hpp | 3 +++
5 files changed, 62 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/cc395509/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index db4fc8c..15f55a3 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -753,14 +753,17 @@ enum TaskState {
* Describes the current status of a task.
*/
message TaskStatus {
- /** Describes the source of the task status update. */
+ // Describes the source of the task status update.
enum Source {
SOURCE_MASTER = 0;
SOURCE_SLAVE = 1;
SOURCE_EXECUTOR = 2;
}
- /** Detailed reason for the task status update. */
+ // Detailed reason for the task status update.
+ //
+ // TODO(bmahler): Differentiate between slave removal reasons
+ // (e.g. unhealthy vs. unregistered for maintenance).
enum Reason {
REASON_COMMAND_EXECUTOR_FAILED = 0;
REASON_EXECUTOR_TERMINATED = 1;
http://git-wip-us.apache.org/repos/asf/mesos/blob/cc395509/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index ec32cd6..eaea79d 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -993,7 +993,8 @@ void Master::exited(const UPID& pid)
// Remove the slave, if it is not checkpointing.
LOG(INFO) << "Removing disconnected slave " << *slave
<< " because it is not checkpointing!";
- removeSlave(slave);
+ removeSlave(slave,
+ "slave is non-checkpointing and disconnected");
return;
} else if (slave->connected) {
// Checkpointing slaves can just be disconnected.
@@ -1380,7 +1381,9 @@ Nothing Master::removeSlave(const Registry::Slave& slave)
&Self::_removeSlave,
slave.info(),
vector<StatusUpdate>(), // No TASK_LOST updates to send.
- lambda::_1));
+ lambda::_1,
+ "did not re-register after master failover",
+ metrics->slave_removals_reason_unhealthy));
} else {
// When a non-strict registry is in use, we want to ensure the
// registry is used in a write-only manner. Therefore we remove
@@ -3101,7 +3104,9 @@ void Master::registerSlave(
LOG(INFO)
<< "Removing old disconnected slave " << *slave
<< " because a registration attempt is being made from " << from;
- removeSlave(slave);
+ removeSlave(slave,
+ "a new slave registered at the same address",
+ metrics->slave_removals_reason_registered);
break;
} else {
CHECK(slave->active)
@@ -3437,7 +3442,9 @@ void Master::unregisterSlave(const UPID& from, const SlaveID& slaveId)
<< " because it is not the slave " << slave->pid;
return;
}
- removeSlave(slave);
+ removeSlave(slave,
+ "the slave unregistered",
+ metrics->slave_removals_reason_unregistered);
}
}
@@ -3646,7 +3653,7 @@ void Master::shutdownSlave(const SlaveID& slaveId, const string& message)
message_.set_message(message);
send(slave->pid, message_);
- removeSlave(slave);
+ removeSlave(slave, message, metrics->slave_removals_reason_unhealthy);
}
@@ -4664,11 +4671,14 @@ void Master::addSlave(
}
-void Master::removeSlave(Slave* slave)
+void Master::removeSlave(
+ Slave* slave,
+ const string& message,
+ Option<Counter> reason)
{
CHECK_NOTNULL(slave);
- LOG(INFO) << "Removing slave " << *slave;
+ LOG(INFO) << "Removing slave " << *slave << ": " << message;
// We want to remove the slave first, to avoid the allocator
// re-allocating the recovered resources.
@@ -4692,7 +4702,7 @@ void Master::removeSlave(Slave* slave)
task->task_id(),
TASK_LOST,
TaskStatus::SOURCE_MASTER,
- "Slave " + slave->info.hostname() + " removed",
+ "Slave " + slave->info.hostname() + " removed: " + message,
TaskStatus::REASON_SLAVE_REMOVED,
(task->has_executor_id() ?
Option<ExecutorID>(task->executor_id()) : None()));
@@ -4743,7 +4753,9 @@ void Master::removeSlave(Slave* slave)
&Self::_removeSlave,
slave->info,
updates,
- lambda::_1));
+ lambda::_1,
+ message,
+ reason));
delete slave;
}
@@ -4752,7 +4764,9 @@ void Master::removeSlave(Slave* slave)
void Master::_removeSlave(
const SlaveInfo& slaveInfo,
const vector<StatusUpdate>& updates,
- const Future<bool>& removed)
+ const Future<bool>& removed,
+ const string& message,
+ Option<Counter> reason)
{
slaves.removing.erase(slaveInfo.id());
@@ -4769,9 +4783,14 @@ void Master::_removeSlave(
<< "already removed from the registrar";
LOG(INFO) << "Removed slave " << slaveInfo.id() << " ("
- << slaveInfo.hostname() << ")";
+ << slaveInfo.hostname() << "): " << message;
+
++metrics->slave_removals;
+ if (reason.isSome()) {
+ ++utils::copy(reason.get()); // Remove const.
+ }
+
// Forward the LOST updates on to the framework.
foreach (const StatusUpdate& update, updates) {
Framework* framework = getFramework(update.framework_id());
http://git-wip-us.apache.org/repos/asf/mesos/blob/cc395509/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index af41216..da0a835 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -44,6 +44,8 @@
#include <process/protobuf.hpp>
#include <process/timer.hpp>
+#include <process/metrics/counter.hpp>
+
#include <stout/cache.hpp>
#include <stout/foreach.hpp>
#include <stout/hashmap.hpp>
@@ -363,11 +365,19 @@ protected:
Nothing removeSlave(const Registry::Slave& slave);
// Remove the slave from the registrar and from the master's state.
- void removeSlave(Slave* slave);
+ //
+ // TODO(bmahler): 'reason' is optional until MESOS-2317 is resolved.
+ void removeSlave(
+ Slave* slave,
+ const std::string& message,
+ Option<process::metrics::Counter> reason = None());
+
void _removeSlave(
const SlaveInfo& slaveInfo,
const std::vector<StatusUpdate>& updates,
- const process::Future<bool>& removed);
+ const process::Future<bool>& removed,
+ const std::string& message,
+ Option<process::metrics::Counter> reason = None());
// Authorizes the task.
// Returns true if task is authorized.
http://git-wip-us.apache.org/repos/asf/mesos/blob/cc395509/src/master/metrics.cpp
----------------------------------------------------------------------
diff --git a/src/master/metrics.cpp b/src/master/metrics.cpp
index 973f051..ee09664 100644
--- a/src/master/metrics.cpp
+++ b/src/master/metrics.cpp
@@ -148,6 +148,12 @@ Metrics::Metrics(const Master& master)
"master/slave_reregistrations"),
slave_removals(
"master/slave_removals"),
+ slave_removals_reason_unhealthy(
+ "master/slave_removals/reason_unhealthy"),
+ slave_removals_reason_unregistered(
+ "master/slave_removals/reason_unregistered"),
+ slave_removals_reason_registered(
+ "master/slave_removals/reason_registered"),
slave_shutdowns_scheduled(
"master/slave_shutdowns_scheduled"),
slave_shutdowns_completed(
@@ -224,6 +230,9 @@ Metrics::Metrics(const Master& master)
process::metrics::add(slave_registrations);
process::metrics::add(slave_reregistrations);
process::metrics::add(slave_removals);
+ process::metrics::add(slave_removals_reason_unhealthy);
+ process::metrics::add(slave_removals_reason_unregistered);
+ process::metrics::add(slave_removals_reason_registered);
process::metrics::add(slave_shutdowns_scheduled);
process::metrics::add(slave_shutdowns_completed);
@@ -327,6 +336,9 @@ Metrics::~Metrics()
process::metrics::remove(slave_registrations);
process::metrics::remove(slave_reregistrations);
process::metrics::remove(slave_removals);
+ process::metrics::remove(slave_removals_reason_unhealthy);
+ process::metrics::remove(slave_removals_reason_unregistered);
+ process::metrics::remove(slave_removals_reason_registered);
process::metrics::remove(slave_shutdowns_scheduled);
process::metrics::remove(slave_shutdowns_completed);
http://git-wip-us.apache.org/repos/asf/mesos/blob/cc395509/src/master/metrics.hpp
----------------------------------------------------------------------
diff --git a/src/master/metrics.hpp b/src/master/metrics.hpp
index ee3982e..78d0666 100644
--- a/src/master/metrics.hpp
+++ b/src/master/metrics.hpp
@@ -163,6 +163,9 @@ struct Metrics
process::metrics::Counter slave_registrations;
process::metrics::Counter slave_reregistrations;
process::metrics::Counter slave_removals;
+ process::metrics::Counter slave_removals_reason_unhealthy;
+ process::metrics::Counter slave_removals_reason_unregistered;
+ process::metrics::Counter slave_removals_reason_registered;
// Slave observer metrics.
process::metrics::Counter slave_shutdowns_scheduled;
[4/4] mesos git commit: Moved the slave shutdown test into
slave_tests.cpp.
Posted by bm...@apache.org.
Moved the slave shutdown test into slave_tests.cpp.
Review: https://reviews.apache.org/r/33152
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/085c75cb
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/085c75cb
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/085c75cb
Branch: refs/heads/master
Commit: 085c75cb7447d594a1418e048c877601de75131e
Parents: 288e588
Author: Benjamin Mahler <be...@gmail.com>
Authored: Mon Apr 13 17:32:22 2015 -0700
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Wed May 13 16:45:30 2015 -0700
----------------------------------------------------------------------
src/tests/fault_tolerance_tests.cpp | 48 --------------------------------
src/tests/slave_tests.cpp | 47 +++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/085c75cb/src/tests/fault_tolerance_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fault_tolerance_tests.cpp b/src/tests/fault_tolerance_tests.cpp
index 3a27d82..cd2594b 100644
--- a/src/tests/fault_tolerance_tests.cpp
+++ b/src/tests/fault_tolerance_tests.cpp
@@ -86,54 +86,6 @@ namespace tests {
class FaultToleranceTest : public MesosTest {};
-// This test checks that when a slave is lost,
-// its offer(s) is rescinded.
-TEST_F(FaultToleranceTest, SlaveLost)
-{
- Try<PID<Master> > master = StartMaster();
- ASSERT_SOME(master);
-
- Try<PID<Slave> > slave = StartSlave();
- ASSERT_SOME(slave);
-
- MockScheduler sched;
- MesosSchedulerDriver driver(
- &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL);
-
- EXPECT_CALL(sched, registered(&driver, _, _));
-
- Future<vector<Offer> > offers;
- EXPECT_CALL(sched, resourceOffers(&driver, _))
- .WillOnce(FutureArg<1>(&offers))
- .WillRepeatedly(Return()); // Ignore subsequent offers.
-
- driver.start();
-
- AWAIT_READY(offers);
- EXPECT_EQ(1u, offers.get().size());
-
- Future<Nothing> offerRescinded;
- EXPECT_CALL(sched, offerRescinded(&driver, offers.get()[0].id()))
- .WillOnce(FutureSatisfy(&offerRescinded));
-
- Future<Nothing> slaveLost;
- EXPECT_CALL(sched, slaveLost(&driver, offers.get()[0].slave_id()))
- .WillOnce(FutureSatisfy(&slaveLost));
-
- // Stop the checkpointing slave with explicit shutdown message
- // so that the master does not wait for it to reconnect.
- Stop(slave.get(), true);
-
- AWAIT_READY(offerRescinded);
- AWAIT_READY(slaveLost);
-
- driver.stop();
- driver.join();
-
- Shutdown();
-}
-
-
// The purpose of this test is to ensure that when slaves are removed
// from the master, and then attempt to send status updates, we send
// a ShutdownMessage to the slave. Why? Because during a network
http://git-wip-us.apache.org/repos/asf/mesos/blob/085c75cb/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 04e79ec..066cbb0 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -97,6 +97,53 @@ namespace http = process::http;
class SlaveTest : public MesosTest {};
+// This test ensures that when a slave shuts itself down, it
+// unregisters itself and the master notifies the framework
+// immediately and rescinds any offers.
+TEST_F(SlaveTest, Shutdown)
+{
+ Try<PID<Master> > master = StartMaster();
+ ASSERT_SOME(master);
+
+ Try<PID<Slave> > slave = StartSlave();
+ ASSERT_SOME(slave);
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ Future<vector<Offer> > offers;
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ driver.start();
+
+ AWAIT_READY(offers);
+ EXPECT_EQ(1u, offers.get().size());
+
+ Future<Nothing> offerRescinded;
+ EXPECT_CALL(sched, offerRescinded(&driver, offers.get()[0].id()))
+ .WillOnce(FutureSatisfy(&offerRescinded));
+
+ Future<Nothing> slaveLost;
+ EXPECT_CALL(sched, slaveLost(&driver, offers.get()[0].slave_id()))
+ .WillOnce(FutureSatisfy(&slaveLost));
+
+ // Stop the checkpointing slave with explicit shutdown message
+ // so that the slave unregisters.
+ Stop(slave.get(), true);
+
+ AWAIT_READY(offerRescinded);
+ AWAIT_READY(slaveLost);
+
+ driver.stop();
+ driver.join();
+}
+
+
TEST_F(SlaveTest, ShutdownUnregisteredExecutor)
{
Try<PID<Master>> master = StartMaster();
[2/4] mesos git commit: Moved a partition test into
partition_tests.cpp.
Posted by bm...@apache.org.
Moved a partition test into partition_tests.cpp.
Review: https://reviews.apache.org/r/33153
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/23a51be1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/23a51be1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/23a51be1
Branch: refs/heads/master
Commit: 23a51be160d57bccda31ce4e47c8a79d0c363d55
Parents: 085c75c
Author: Benjamin Mahler <be...@gmail.com>
Authored: Mon Apr 13 17:34:30 2015 -0700
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Wed May 13 16:45:30 2015 -0700
----------------------------------------------------------------------
src/tests/fault_tolerance_tests.cpp | 115 ------------------------------
src/tests/partition_tests.cpp | 117 +++++++++++++++++++++++++++++++
2 files changed, 117 insertions(+), 115 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/23a51be1/src/tests/fault_tolerance_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fault_tolerance_tests.cpp b/src/tests/fault_tolerance_tests.cpp
index cd2594b..fb28e2a 100644
--- a/src/tests/fault_tolerance_tests.cpp
+++ b/src/tests/fault_tolerance_tests.cpp
@@ -86,121 +86,6 @@ namespace tests {
class FaultToleranceTest : public MesosTest {};
-// The purpose of this test is to ensure that when slaves are removed
-// from the master, and then attempt to send status updates, we send
-// a ShutdownMessage to the slave. Why? Because during a network
-// partition, the master will remove a partitioned slave, thus sending
-// its tasks to LOST. At this point, when the partition is removed,
-// the slave may attempt to send updates if it was unaware that the
-// master removed it. We've already notified frameworks that these
-// tasks were LOST, so we have to have the slave shut down.
-TEST_F(FaultToleranceTest, PartitionedSlaveStatusUpdates)
-{
- Try<PID<Master> > master = StartMaster();
- ASSERT_SOME(master);
-
- // Allow the master to PING the slave, but drop all PONG messages
- // from the slave. Note that we don't match on the master / slave
- // PIDs because it's actually the SlaveObserver Process that sends
- // the pings.
- Future<Message> ping = FUTURE_MESSAGE(Eq("PING"), _, _);
- DROP_MESSAGES(Eq("PONG"), _, _);
-
- Future<SlaveRegisteredMessage> slaveRegisteredMessage =
- FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
-
- MockExecutor exec(DEFAULT_EXECUTOR_ID);
-
- Try<PID<Slave> > slave = StartSlave(&exec);
- ASSERT_SOME(slave);
-
- AWAIT_READY(slaveRegisteredMessage);
- SlaveID slaveId = slaveRegisteredMessage.get().slave_id();
-
- MockScheduler sched;
- MesosSchedulerDriver driver(
- &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL);
-
- Future<FrameworkID> frameworkId;
- EXPECT_CALL(sched, registered(&driver, _, _))
- .WillOnce(FutureArg<1>(&frameworkId));
-
- EXPECT_CALL(sched, resourceOffers(&driver, _))
- .WillRepeatedly(Return());
-
- driver.start();
-
- AWAIT_READY(frameworkId);
-
- // Drop the first shutdown message from the master (simulated
- // partition), allow the second shutdown message to pass when
- // the slave sends an update.
- Future<ShutdownMessage> shutdownMessage =
- DROP_PROTOBUF(ShutdownMessage(), _, slave.get());
-
- EXPECT_CALL(sched, offerRescinded(&driver, _))
- .WillRepeatedly(Return());
-
- Future<Nothing> slaveLost;
- EXPECT_CALL(sched, slaveLost(&driver, _))
- .WillOnce(FutureSatisfy(&slaveLost));
-
- Clock::pause();
-
- // Now, induce a partition of the slave by having the master
- // timeout the slave.
- uint32_t pings = 0;
- while (true) {
- AWAIT_READY(ping);
- pings++;
- if (pings == master::MAX_SLAVE_PING_TIMEOUTS) {
- break;
- }
- ping = FUTURE_MESSAGE(Eq("PING"), _, _);
- Clock::advance(master::SLAVE_PING_TIMEOUT);
- Clock::settle();
- }
-
- Clock::advance(master::SLAVE_PING_TIMEOUT);
- Clock::settle();
-
- // Wait for the master to attempt to shut down the slave.
- AWAIT_READY(shutdownMessage);
-
- // The master will notify the framework that the slave was lost.
- AWAIT_READY(slaveLost);
-
- shutdownMessage = FUTURE_PROTOBUF(ShutdownMessage(), _, slave.get());
-
- // At this point, the slave still thinks it's registered, so we
- // simulate a status update coming from the slave.
- TaskID taskId;
- taskId.set_value("task_id");
- const StatusUpdate& update = createStatusUpdate(
- frameworkId.get(),
- slaveId,
- taskId,
- TASK_RUNNING,
- TaskStatus::SOURCE_SLAVE);
-
- StatusUpdateMessage message;
- message.mutable_update()->CopyFrom(update);
- message.set_pid(stringify(slave.get()));
-
- process::post(master.get(), message);
-
- // The master should shutdown the slave upon receiving the update.
- AWAIT_READY(shutdownMessage);
-
- Clock::resume();
-
- driver.stop();
- driver.join();
-
- Shutdown();
-}
-
-
// This test ensures that a framework connecting with a
// failed over master gets a registered callback.
// Note that this behavior might change in the future and
http://git-wip-us.apache.org/repos/asf/mesos/blob/23a51be1/src/tests/partition_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/partition_tests.cpp b/src/tests/partition_tests.cpp
index 1018e47..5e31cac 100644
--- a/src/tests/partition_tests.cpp
+++ b/src/tests/partition_tests.cpp
@@ -27,6 +27,8 @@
#include <stout/try.hpp>
+#include "common/protobuf_utils.hpp"
+
#include "master/master.hpp"
#include "slave/constants.hpp"
@@ -284,6 +286,121 @@ TEST_F(PartitionTest, PartitionedSlaveReregistration)
// The purpose of this test is to ensure that when slaves are removed
+// from the master, and then attempt to send status updates, we send
+// a ShutdownMessage to the slave. Why? Because during a network
+// partition, the master will remove a partitioned slave, thus sending
+// its tasks to LOST. At this point, when the partition is removed,
+// the slave may attempt to send updates if it was unaware that the
+// master removed it. We've already notified frameworks that these
+// tasks were LOST, so we have to have the slave shut down.
+TEST_F(PartitionTest, PartitionedSlaveStatusUpdates)
+{
+ Try<PID<Master> > master = StartMaster();
+ ASSERT_SOME(master);
+
+ // Allow the master to PING the slave, but drop all PONG messages
+ // from the slave. Note that we don't match on the master / slave
+ // PIDs because it's actually the SlaveObserver Process that sends
+ // the pings.
+ Future<Message> ping = FUTURE_MESSAGE(Eq("PING"), _, _);
+ DROP_MESSAGES(Eq("PONG"), _, _);
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ MockExecutor exec(DEFAULT_EXECUTOR_ID);
+
+ Try<PID<Slave> > slave = StartSlave(&exec);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+ SlaveID slaveId = slaveRegisteredMessage.get().slave_id();
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL);
+
+ Future<FrameworkID> frameworkId;
+ EXPECT_CALL(sched, registered(&driver, _, _))
+ .WillOnce(FutureArg<1>(&frameworkId));
+
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillRepeatedly(Return());
+
+ driver.start();
+
+ AWAIT_READY(frameworkId);
+
+ // Drop the first shutdown message from the master (simulated
+ // partition), allow the second shutdown message to pass when
+ // the slave sends an update.
+ Future<ShutdownMessage> shutdownMessage =
+ DROP_PROTOBUF(ShutdownMessage(), _, slave.get());
+
+ EXPECT_CALL(sched, offerRescinded(&driver, _))
+ .WillRepeatedly(Return());
+
+ Future<Nothing> slaveLost;
+ EXPECT_CALL(sched, slaveLost(&driver, _))
+ .WillOnce(FutureSatisfy(&slaveLost));
+
+ Clock::pause();
+
+ // Now, induce a partition of the slave by having the master
+ // timeout the slave.
+ uint32_t pings = 0;
+ while (true) {
+ AWAIT_READY(ping);
+ pings++;
+ if (pings == master::MAX_SLAVE_PING_TIMEOUTS) {
+ break;
+ }
+ ping = FUTURE_MESSAGE(Eq("PING"), _, _);
+ Clock::advance(master::SLAVE_PING_TIMEOUT);
+ Clock::settle();
+ }
+
+ Clock::advance(master::SLAVE_PING_TIMEOUT);
+ Clock::settle();
+
+ // Wait for the master to attempt to shut down the slave.
+ AWAIT_READY(shutdownMessage);
+
+ // The master will notify the framework that the slave was lost.
+ AWAIT_READY(slaveLost);
+
+ shutdownMessage = FUTURE_PROTOBUF(ShutdownMessage(), _, slave.get());
+
+ // At this point, the slave still thinks it's registered, so we
+ // simulate a status update coming from the slave.
+ TaskID taskId;
+ taskId.set_value("task_id");
+ const StatusUpdate& update = protobuf::createStatusUpdate(
+ frameworkId.get(),
+ slaveId,
+ taskId,
+ TASK_RUNNING,
+ TaskStatus::SOURCE_SLAVE);
+
+ StatusUpdateMessage message;
+ message.mutable_update()->CopyFrom(update);
+ message.set_pid(stringify(slave.get()));
+
+ process::post(master.get(), message);
+
+ // The master should shutdown the slave upon receiving the update.
+ AWAIT_READY(shutdownMessage);
+
+ Clock::resume();
+
+ driver.stop();
+ driver.join();
+
+ Shutdown();
+}
+
+
+// The purpose of this test is to ensure that when slaves are removed
// from the master, and then attempt to send exited executor messages,
// we send a ShutdownMessage to the slave. Why? Because during a
// network partition, the master will remove a partitioned slave, thus