Fixed the implementation of per-role Suppress / Revive calls.

The existing implementation uses a single suppression boolean which
results in incorrect handling of per-role suppression / revival.

See: https://issues.apache.org/jira/browse/MESOS-7430

Review: https://reviews.apache.org/r/58829


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/786f9f67
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/786f9f67
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/786f9f67

Branch: refs/heads/master
Commit: 786f9f6771988719cd6e0dde2ba333d1717fccfb
Parents: 6f6a586
Author: Benjamin Mahler <[email protected]>
Authored: Thu Apr 27 17:34:03 2017 -0700
Committer: Benjamin Mahler <[email protected]>
Committed: Fri Apr 28 15:59:34 2017 -0700

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 22 ++++++----------------
 src/master/allocator/mesos/hierarchical.hpp |  3 ---
 2 files changed, 6 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/786f9f67/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp 
b/src/master/allocator/mesos/hierarchical.cpp
index 84dc31d..ce01128 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -127,7 +127,6 @@ private:
 HierarchicalAllocatorProcess::Framework::Framework(
     const FrameworkInfo& frameworkInfo)
   : roles(protobuf::framework::getRoles(frameworkInfo)),
-    suppressed(false),
     capabilities(frameworkInfo.capabilities()) {}
 
 
@@ -372,10 +371,6 @@ void HierarchicalAllocatorProcess::deactivateFramework(
   framework.offerFilters.clear();
   framework.inverseOfferFilters.clear();
 
-  // Clear the suppressed flag to make sure the framework can be offered
-  // resources immediately after getting activated.
-  framework.suppressed = false;
-
   LOG(INFO) << "Deactivated framework " << frameworkId;
 }
 
@@ -1206,7 +1201,6 @@ void HierarchicalAllocatorProcess::suppressOffers(
   CHECK(frameworks.contains(frameworkId));
 
   Framework& framework = frameworks.at(frameworkId);
-  framework.suppressed = true;
 
   // Deactivating the framework in the sorter is fine as long as
   // SUPPRESS is not parameterized. When parameterization is added,
@@ -1238,16 +1232,12 @@ void HierarchicalAllocatorProcess::reviveOffers(
   const set<string>& roles =
     role.isSome() ? set<string>{role.get()} : framework.roles;
 
-  if (framework.suppressed) {
-    framework.suppressed = false;
-
-    // Activating the framework in the sorter on REVIVE 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) {
-      CHECK(frameworkSorters.contains(role));
-      frameworkSorters.at(role)->activate(frameworkId.value());
-    }
+  // Activating the framework in the sorter on REVIVE 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) {
+    CHECK(frameworkSorters.contains(role));
+    frameworkSorters.at(role)->activate(frameworkId.value());
   }
 
   // We delete each actual `OfferFilter` when

http://git-wip-us.apache.org/repos/asf/mesos/blob/786f9f67/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp 
b/src/master/allocator/mesos/hierarchical.hpp
index 219f508..79420fa 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -302,9 +302,6 @@ protected:
 
     std::set<std::string> roles;
 
-    // Whether the framework suppresses offers.
-    bool suppressed;
-
     protobuf::framework::Capabilities capabilities;
 
     // Active offer and inverse offer filters for the framework.

Reply via email to