[mesos] 01/02: Cleared filters upon unsuppressing a role.

[bm...@apache.org]

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

commit 28a0bc2284049e9a4765fd371f0c8a1293ff79c5
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Fri Aug 9 17:31:55 2019 -0400

    Cleared filters upon unsuppressing a role.
    
    Per MESOS-9932, removal of a role from the suppression list does
    not currently clear filters. This means that schedulers have to
    issue a separate explicit REVIVE for the roles they want to remove.
    
    This ensures that filters are cleared upon removing a role from
    the suppression list.
    
    Review: https://reviews.apache.org/r/71264
---
 src/master/allocator/mesos/hierarchical.cpp | 33 +++++++++++++++--------------
 src/master/allocator/mesos/hierarchical.hpp |  2 +-
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index f1fa838..87b03d3 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -499,6 +499,8 @@ void HierarchicalAllocatorProcess::updateFramework(
     if (!isFrameworkTrackedUnderRole(frameworkId, role)) {
       trackFrameworkUnderRole(frameworkId, role);
     }
+
+    frameworkSorters.at(role)->activate(frameworkId.value());
   }
 
   foreach (const string& role, oldRoles - newRoles) {
@@ -525,8 +527,10 @@ void HierarchicalAllocatorProcess::updateFramework(
   framework.minAllocatableResources =
     unpackFrameworkOfferFilters(frameworkInfo.offer_filters());
 
-  suppressRoles(frameworkId, suppressedRoles);
-  unsuppressRoles(frameworkId, newRoles - suppressedRoles);
+  suppressRoles(
+      frameworkId, suppressedRoles - oldSuppressedRoles);
+  reviveRoles(
+      frameworkId, (oldSuppressedRoles - suppressedRoles) & newRoles);
 
   CHECK(framework.suppressedRoles == suppressedRoles)
     << "After updating framework " << frameworkId
@@ -1340,7 +1344,7 @@ void HierarchicalAllocatorProcess::suppressOffers(
 }
 
 
-void HierarchicalAllocatorProcess::unsuppressRoles(
+void HierarchicalAllocatorProcess::reviveRoles(
     const FrameworkID& frameworkId,
     const set<string>& roles)
 {
@@ -1349,6 +1353,12 @@ void HierarchicalAllocatorProcess::unsuppressRoles(
 
   Framework& framework = frameworks.at(frameworkId);
 
+  framework.inverseOfferFilters.clear();
+
+  foreach (const string& role, roles) {
+    framework.offerFilters.erase(role);
+  }
+
   // 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.
@@ -1362,30 +1372,21 @@ void HierarchicalAllocatorProcess::unsuppressRoles(
 
   // 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;
+  LOG(INFO) << "Unsuppressed offers and cleared filters for roles "
+            << stringify(roles) << " of framework " << frameworkId;
 }
 
 
 void HierarchicalAllocatorProcess::reviveOffers(
     const FrameworkID& frameworkId,
-    const set<string>& roles_)
+    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;
+  reviveRoles(frameworkId, roles.empty() ? framework.roles : roles);
 
   allocate();
 }
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 29dfa9f..dee5baf 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -638,7 +638,7 @@ private:
   void suppressRoles(
       const FrameworkID& frameworkId,
       const std::set<std::string>& roles);
-  void unsuppressRoles(
+  void reviveRoles(
       const FrameworkID& frameworkId,
       const std::set<std::string>& roles);