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 2016/04/25 23:15:22 UTC
[06/24] mesos git commit: Maintenance: Fixed bug that sent two
inverse offers.
Maintenance: Fixed bug that sent two inverse offers.
Review: https://reviews.apache.org/r/44258/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2f58264d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2f58264d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2f58264d
Branch: refs/heads/0.28.x
Commit: 2f58264d6a89899622152bec0b1723a97ef01074
Parents: f28cc0f
Author: Guangya Liu <gy...@gmail.com>
Authored: Thu Mar 10 19:29:37 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Mar 11 13:47:51 2016 -0500
----------------------------------------------------------------------
src/master/http.cpp | 25 +++++++++++++++----
src/tests/master_maintenance_tests.cpp | 37 ++++++++++++-----------------
2 files changed, 35 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2f58264d/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 8276baa..b2bc2ae 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2013,19 +2013,29 @@ Future<Response> Master::Http::maintenanceSchedule(const Request& request) const
// Put the machines in the updated schedule into a set.
// Save the unavailability, to help with updating some machines.
- hashmap<MachineID, Unavailability> updated;
+ hashmap<MachineID, Unavailability> unavailabilities;
foreach (const mesos::maintenance::Window& window, schedule.windows()) {
foreach (const MachineID& id, window.machine_ids()) {
- updated[id] = window.unavailability();
+ unavailabilities[id] = window.unavailability();
}
}
// NOTE: Copies are needed because `updateUnavailability()` in this loop
// modifies the container.
foreachkey (const MachineID& id, utils::copy(master->machines)) {
- // Update the entry for each updated machine.
- if (updated.contains(id)) {
- master->updateUnavailability(id, updated[id]);
+ // Update the `unavailability` for each existing machine, except for
+ // machines going from `UP` to `DRAINING` (handled in the next loop).
+ // Each machine will only be touched by 1 of the 2 loops here to
+ // avoid sending inverse offer twice for a single machine since
+ // `updateUnavailability` will trigger an inverse offer.
+ // TODO(gyliu513): Merge this logic with `Master::updateUnavailability`,
+ // having it in two places results in more conditionals to handle.
+ if (unavailabilities.contains(id)) {
+ if (master->machines[id].info.mode() == MachineInfo::UP) {
+ continue;
+ }
+
+ master->updateUnavailability(id, unavailabilities[id]);
continue;
}
@@ -2039,6 +2049,11 @@ Future<Response> Master::Http::maintenanceSchedule(const Request& request) const
// and starting in `DRAINING` mode.
foreach (const mesos::maintenance::Window& window, schedule.windows()) {
foreach (const MachineID& id, window.machine_ids()) {
+ if (master->machines.contains(id) &&
+ master->machines[id].info.mode() != MachineInfo::UP) {
+ continue;
+ }
+
MachineInfo info;
info.mutable_id()->CopyFrom(id);
info.set_mode(MachineInfo::DRAINING);
http://git-wip-us.apache.org/repos/asf/mesos/blob/2f58264d/src/tests/master_maintenance_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_maintenance_tests.cpp b/src/tests/master_maintenance_tests.cpp
index 3faa813..6e5c576 100644
--- a/src/tests/master_maintenance_tests.cpp
+++ b/src/tests/master_maintenance_tests.cpp
@@ -421,13 +421,10 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
event = events.get();
AWAIT_READY(event);
EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_NE(0, event.get().offers().offers().size());
- const size_t numberOfOffers = event.get().offers().offers().size();
+ EXPECT_EQ(1, event.get().offers().offers().size());
// Regular offers shouldn't have unavailability.
- foreach (const v1::Offer& offer, event.get().offers().offers()) {
- EXPECT_FALSE(offer.has_unavailability());
- }
+ EXPECT_FALSE(event.get().offers().offers(0).has_unavailability());
// Schedule this slave for maintenance.
MachineID machine;
@@ -457,35 +454,31 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
// The original offers should be rescinded when the unavailability
// is changed. We expect as many rescind events as we received
// original offers.
- for (size_t offerNumber = 0; offerNumber < numberOfOffers; ++offerNumber) {
- event = events.get();
- AWAIT_READY(event);
- EXPECT_EQ(Event::RESCIND, event.get().type());
- }
+ event = events.get();
+ AWAIT_READY(event);
+ EXPECT_EQ(Event::RESCIND, event.get().type());
event = events.get();
AWAIT_READY(event);
EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_NE(0, event.get().offers().offers().size());
+ EXPECT_EQ(1, event.get().offers().offers().size());
- // Make sure the new offers have the unavailability set.
- foreach (const v1::Offer& offer, event.get().offers().offers()) {
- EXPECT_TRUE(offer.has_unavailability());
- EXPECT_EQ(
- unavailability.start().nanoseconds(),
- offer.unavailability().start().nanoseconds());
+ v1::Offer offer = event.get().offers().offers(0);
+ EXPECT_TRUE(offer.has_unavailability());
+ EXPECT_EQ(
+ unavailability.start().nanoseconds(),
+ offer.unavailability().start().nanoseconds());
- EXPECT_EQ(
- unavailability.duration().nanoseconds(),
- offer.unavailability().duration().nanoseconds());
- }
+ EXPECT_EQ(
+ unavailability.duration().nanoseconds(),
+ offer.unavailability().duration().nanoseconds());
// We also expect an inverse offer for the slave to go under
// maintenance.
event = events.get();
AWAIT_READY(event);
EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_NE(0, event.get().offers().inverse_offers().size());
+ EXPECT_EQ(1, event.get().offers().inverse_offers().size());
EXPECT_CALL(exec, shutdown(_))
.Times(AtMost(1));