You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gy...@apache.org on 2017/02/09 06:41:14 UTC
mesos git commit: Enabled suppress offer per role.
Repository: mesos
Updated Branches:
refs/heads/master 792e1a617 -> 4fb2a5d2e
Enabled suppress offer per role.
Enabled suppress offer per role.
Review: https://reviews.apache.org/r/56330/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4fb2a5d2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4fb2a5d2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4fb2a5d2
Branch: refs/heads/master
Commit: 4fb2a5d2edeca0966c0f3ea3445f9723d0140d09
Parents: 792e1a6
Author: Guangya Liu <gy...@apache.org>
Authored: Thu Feb 9 14:40:04 2017 +0800
Committer: Guangya Liu <gy...@gmail.com>
Committed: Thu Feb 9 14:40:42 2017 +0800
----------------------------------------------------------------------
include/mesos/allocator/allocator.hpp | 11 +++++---
src/master/allocator/mesos/allocator.hpp | 12 ++++++---
src/master/allocator/mesos/hierarchical.cpp | 10 ++++---
src/master/allocator/mesos/hierarchical.hpp | 4 +--
src/master/master.cpp | 33 +++++++++++++++++++++++-
src/tests/allocator.hpp | 11 ++++----
src/tests/hierarchical_allocator_tests.cpp | 6 ++---
7 files changed, 65 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fb2a5d2/include/mesos/allocator/allocator.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/allocator/allocator.hpp b/include/mesos/allocator/allocator.hpp
index 71a4053..449a1ca 100644
--- a/include/mesos/allocator/allocator.hpp
+++ b/include/mesos/allocator/allocator.hpp
@@ -347,13 +347,16 @@ public:
/**
* Suppresses offers.
*
- * Informs the allocator to stop sending offers to the framework.
+ * Informs the allocator to stop sending offers to this framework for the
+ * specified role. If the role is not specified, we will stop sending offers
+ * to this framework for all of its roles.
*
- * TODO(bmahler): Take an optional role to allow frameworks with
- * multiple roles to do fine-grained suppression.
+ * @param role The optional role parameter allows frameworks with multiple
+ * roles to do fine-grained suppression.
*/
virtual void suppressOffers(
- const FrameworkID& frameworkId) = 0;
+ const FrameworkID& frameworkId,
+ const Option<std::string>& role) = 0;
/**
* Revives offers for a framework. This is invoked by a framework when
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fb2a5d2/src/master/allocator/mesos/allocator.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/allocator.hpp b/src/master/allocator/mesos/allocator.hpp
index e3c8618..fd36b39 100644
--- a/src/master/allocator/mesos/allocator.hpp
+++ b/src/master/allocator/mesos/allocator.hpp
@@ -143,7 +143,8 @@ public:
const Option<Filters>& filters);
void suppressOffers(
- const FrameworkID& frameworkId);
+ const FrameworkID& frameworkId,
+ const Option<std::string>& role);
void reviveOffers(
const FrameworkID& frameworkId);
@@ -276,7 +277,8 @@ public:
const Option<Filters>& filters) = 0;
virtual void suppressOffers(
- const FrameworkID& frameworkId) = 0;
+ const FrameworkID& frameworkId,
+ const Option<std::string>& role) = 0;
virtual void reviveOffers(
const FrameworkID& frameworkId) = 0;
@@ -603,12 +605,14 @@ inline void MesosAllocator<AllocatorProcess>::recoverResources(
template <typename AllocatorProcess>
inline void MesosAllocator<AllocatorProcess>::suppressOffers(
- const FrameworkID& frameworkId)
+ const FrameworkID& frameworkId,
+ const Option<std::string>& role)
{
process::dispatch(
process,
&MesosAllocatorProcess::suppressOffers,
- frameworkId);
+ frameworkId,
+ role);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fb2a5d2/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 017253c..63f4c6b 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1144,7 +1144,8 @@ void HierarchicalAllocatorProcess::recoverResources(
void HierarchicalAllocatorProcess::suppressOffers(
- const FrameworkID& frameworkId)
+ const FrameworkID& frameworkId,
+ const Option<string>& role)
{
CHECK(initialized);
CHECK(frameworks.contains(frameworkId));
@@ -1155,12 +1156,15 @@ void HierarchicalAllocatorProcess::suppressOffers(
// Deactivating the framework in the sorter is fine as long as
// SUPPRESS is not parameterized. When parameterization is added,
// we have to differentiate between the cases here.
- foreach (const string& role, framework.roles) {
+ const set<string>& roles =
+ role.isSome() ? set<string>{role.get()} : framework.roles;
+
+ foreach (const string& role, roles) {
CHECK(frameworkSorters.contains(role));
frameworkSorters.at(role)->deactivate(frameworkId.value());
}
- LOG(INFO) << "Suppressed offers for roles " << stringify(framework.roles)
+ LOG(INFO) << "Suppressed offers for roles " << stringify(roles)
<< " of framework " << frameworkId;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fb2a5d2/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 896abcd..6658d39 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -187,9 +187,9 @@ public:
const Resources& resources,
const Option<Filters>& filters);
- // TODO(bmahler): Update to take optional Suppress.role.
void suppressOffers(
- const FrameworkID& frameworkId);
+ const FrameworkID& frameworkId,
+ const Option<std::string>& role);
// TODO(bmahler): Update to take optional Revive.role.
void reviveOffers(
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fb2a5d2/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 0b65345..20c424a 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -3221,7 +3221,38 @@ void Master::suppress(
++metrics->messages_suppress_offers;
- allocator->suppressOffers(framework->id());
+ const Option<string> role =
+ suppress.has_role() ? Option<string>(suppress.role()) : None();
+
+ // Validate role if it is set. We need to make sure the role is valid
+ // and also one of the framework roles.
+ if (role.isSome()) {
+ // There maybe cases that the framework developer set an invalid role
+ // when constructing `scheduler::Call::Suppress`.
+ Option<Error> roleError = roles::validate(role.get());
+ if (roleError.isSome()) {
+ LOG(WARNING) << "SUPPRESS call message with invalid role: "
+ << roleError.get().message;
+
+ return;
+ }
+
+ // TODO(gyliu513): Store the roles set within the Framework struct, so
+ // that we don't have to keep re-computing it.
+ const set<string> roles = protobuf::framework::getRoles(framework->info);
+ if (roles.count(role.get()) == 0) {
+ // TODO(gyliu513): Consider adding a `drop` overload to avoid
+ // customlogging here.
+ LOG(WARNING)
+ << "Ignoring SUPPRESS call message for framework " << *framework
+ << " with role " << role.get() << " because it is not one of the"
+ << " framework's subscribed roles";
+
+ return;
+ }
+ }
+
+ allocator->suppressOffers(framework->id(), role);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fb2a5d2/src/tests/allocator.hpp
----------------------------------------------------------------------
diff --git a/src/tests/allocator.hpp b/src/tests/allocator.hpp
index 32c2912..64311c2 100644
--- a/src/tests/allocator.hpp
+++ b/src/tests/allocator.hpp
@@ -174,7 +174,7 @@ ACTION_P2(InvokeRecoverResourcesWithFilters, allocator, timeout)
ACTION_P(InvokeSuppressOffers, allocator)
{
- allocator->real->suppressOffers(arg0);
+ allocator->real->suppressOffers(arg0, arg1);
}
@@ -329,9 +329,9 @@ public:
EXPECT_CALL(*this, recoverResources(_, _, _, _))
.WillRepeatedly(DoDefault());
- ON_CALL(*this, suppressOffers(_))
+ ON_CALL(*this, suppressOffers(_, _))
.WillByDefault(InvokeSuppressOffers(this));
- EXPECT_CALL(*this, suppressOffers(_))
+ EXPECT_CALL(*this, suppressOffers(_, _))
.WillRepeatedly(DoDefault());
ON_CALL(*this, reviveOffers(_))
@@ -450,8 +450,9 @@ public:
const Resources&,
const Option<Filters>& filters));
- MOCK_METHOD1(suppressOffers, void(
- const FrameworkID&));
+ MOCK_METHOD2(suppressOffers, void(
+ const FrameworkID&,
+ const Option<std::string>&));
MOCK_METHOD1(reviveOffers, void(
const FrameworkID&));
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fb2a5d2/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index c681d03..44685b8 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -2813,7 +2813,7 @@ TEST_F(HierarchicalAllocatorTest, DeactivateAndReactivateFramework)
None());
// Suppress offers and disconnect framework.
- allocator->suppressOffers(framework.id());
+ allocator->suppressOffers(framework.id(), None());
allocator->deactivateFramework(framework.id());
// Advance the clock and trigger a background allocation cycle.
@@ -2881,7 +2881,7 @@ TEST_F(HierarchicalAllocatorTest, SuppressAndReviveOffers)
allocatedResources(agent.resources(), "role1"),
None());
- allocator->suppressOffers(framework.id());
+ allocator->suppressOffers(framework.id(), None());
// Advance the clock and trigger a background allocation cycle.
Clock::advance(flags.allocation_interval);
@@ -4401,7 +4401,7 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, SuppressOffers)
// 'frameworkCount % allocationsCount' of frameworks not suppressed. For
// the purposes of the benchmark this is not an issue.
for (size_t j = 0; j < frameworkCount / allocationsCount; ++j) {
- allocator->suppressOffers(frameworks[suppressCount].id());
+ allocator->suppressOffers(frameworks[suppressCount].id(), None());
++suppressCount;
}