This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new 0b48116 Extracted suppression logic in allocator for use in update
framework.
0b48116 is described below
commit 0b481168eee979c7272939669483077e20edb8ca
Author: Andrei Sekretenko <[email protected]>
AuthorDate: Wed Jul 3 16:08:33 2019 -0400
Extracted suppression logic in allocator for use in update framework.
This patch moves the logic of suppressing/unsuppressing a role set from
the inside of 'suppressOffers()'/'reviveOffers()' into separate methods.
Specifically, 'reviveOffers()' includes filter clearing logic that we
don't want when unsuppressing roles during framework update. For
'supppressOffers()', we need the empty set == all roles semantics, but
we don't want that in the suppression logic during framework update.
Longer term, the empty set == all roles semantics could be done in
the master and we won't need the extra function to provide empty set
== all roles logic in the allocator.
This is a prerequisite for using thes methods to fix
'updateFramework()' in a subsequent patch.
Review: https://reviews.apache.org/r/70994/
---
src/master/allocator/mesos/hierarchical.cpp | 55 +++++++++++++++++++++++------
src/master/allocator/mesos/hierarchical.hpp | 10 ++++++
2 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/src/master/allocator/mesos/hierarchical.cpp
b/src/master/allocator/mesos/hierarchical.cpp
index 26aad67..c2333bc 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1338,9 +1338,9 @@ void HierarchicalAllocatorProcess::recoverResources(
}
-void HierarchicalAllocatorProcess::suppressOffers(
+void HierarchicalAllocatorProcess::suppressRoles(
const FrameworkID& frameworkId,
- const set<string>& roles_)
+ const set<string>& roles)
{
CHECK(initialized);
CHECK_CONTAINS(frameworks, frameworkId);
@@ -1350,7 +1350,6 @@ 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.
- const set<string>& roles = roles_.empty() ? framework.roles : roles_;
foreach (const string& role, roles) {
CHECK_CONTAINS(frameworkSorters, role);
@@ -1360,37 +1359,71 @@ void HierarchicalAllocatorProcess::suppressOffers(
framework.metrics->suppressRole(role);
}
+ // TODO(bmahler): This logs roles that were already suppressed,
+ // only log roles that transitioned from unsuppressed -> suppressed.
LOG(INFO) << "Suppressed offers for roles " << stringify(roles)
<< " of framework " << frameworkId;
}
-void HierarchicalAllocatorProcess::reviveOffers(
+void HierarchicalAllocatorProcess::suppressOffers(
const FrameworkID& frameworkId,
const set<string>& roles_)
{
- CHECK(initialized);
CHECK_CONTAINS(frameworks, frameworkId);
Framework& framework = frameworks.at(frameworkId);
- framework.inverseOfferFilters.clear();
const set<string>& roles = roles_.empty() ? framework.roles : roles_;
+ suppressRoles(frameworkId, roles);
+}
+
+
+void HierarchicalAllocatorProcess::unsuppressRoles(
+ const FrameworkID& frameworkId,
+ const set<string>& roles)
+{
+ CHECK(initialized);
+ CHECK_CONTAINS(frameworks, frameworkId);
+
+ Framework& framework = frameworks.at(frameworkId);
- // Activating the framework in the sorter on REVIVE is fine as long as
+ // Activating the framework in the sorter is fine as long as
// SUPPRESS is not parameterized. When parameterization is added,
// we may need to differentiate between the cases here.
foreach (const string& role, roles) {
- framework.offerFilters.erase(role);
-
CHECK_CONTAINS(frameworkSorters, role);
- frameworkSorters.at(role)->activate(frameworkId.value());
+ frameworkSorters.at(role)->activate(frameworkId.value());
framework.suppressedRoles.erase(role);
framework.metrics->reviveRole(role);
}
- LOG(INFO) << "Revived offers for roles " << stringify(roles)
+ // TODO(bmahler): This logs roles that were already unsuppressed,
+ // only log roles that transitioned from suppressed -> unsuppressed.
+ LOG(INFO) << "Unsuppressed offers for roles " << stringify(roles)
+ << " of framework " << frameworkId;
+}
+
+
+void HierarchicalAllocatorProcess::reviveOffers(
+ const FrameworkID& frameworkId,
+ const set<string>& roles_)
+{
+ CHECK(initialized);
+ CHECK_CONTAINS(frameworks, frameworkId);
+
+ Framework& framework = frameworks.at(frameworkId);
+ framework.inverseOfferFilters.clear();
+
+ const set<string>& roles = roles_.empty() ? framework.roles : roles_;
+ foreach (const string& role, roles) {
+ framework.offerFilters.erase(role);
+ }
+
+ unsuppressRoles(frameworkId, roles);
+
+ LOG(INFO) << "Revived roles " << stringify(roles)
<< " of framework " << frameworkId;
allocate();
diff --git a/src/master/allocator/mesos/hierarchical.hpp
b/src/master/allocator/mesos/hierarchical.hpp
index 7e97652..d2658bc 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -632,6 +632,16 @@ private:
const FrameworkID& frameworkId,
const std::string& role);
+ // TODO(bmahler): Take a `Framework*` here (and in other functions!),
+ // it's currently not possible because `Framework` doesn't store the
+ // framework ID.
+ void suppressRoles(
+ const FrameworkID& frameworkId,
+ const std::set<std::string>& roles);
+ void unsuppressRoles(
+ const FrameworkID& frameworkId,
+ const std::set<std::string>& roles);
+
// `trackReservations` and `untrackReservations` are helpers
// to track role resource reservations. We need to keep
// track of reservations to enforce role quota limit
