This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 94ef1ca4eaa81c1d5a2bff7085917e66f60ffd59
Author: Meng Zhu <[email protected]>
AuthorDate: Fri May 24 12:35:35 2019 +0200

    Decoupled quota limits from quota guarantees in the allocator.
    
    Currently, resource quota acts as both guarantees and limits.
    This patch adds a field `quotaLimits` in the role struct.
    This paves the way for enforcing guarantees and limits
    separately in the allocator.
    
    Also added a helper `getLimits` to look up quota limits of a
    given role.
    
    Review: https://reviews.apache.org/r/70716
---
 src/master/allocator/mesos/hierarchical.cpp | 33 ++++++++++++++++++++++-------
 src/master/allocator/mesos/hierarchical.hpp | 12 +++++++----
 2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/src/master/allocator/mesos/hierarchical.cpp 
b/src/master/allocator/mesos/hierarchical.cpp
index a3a64d3..7cc241e 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1409,10 +1409,19 @@ void HierarchicalAllocatorProcess::setQuota(
   // the role is not set. Setting quota differs from updating it because
   // the former moves the role to a different allocation group with a
   // dedicated sorter, while the later just updates the actual quota.
-  CHECK(getGuarantees(role).empty());
+  CHECK(getGuarantees(role).empty() && getLimits(role).empty());
+
+  // In the legacy `SetQuota` call, quota guarantee also acts as quota limits.
+  //
+  // Note, quota has been validated to have no duplicate resource names.
+  roles[role].quotaLimits = [&]() {
+    google::protobuf::Map<string, Value::Scalar> limits;
+    foreach (const Resource& r, quota.info.guarantee()) {
+      limits[r.name()] = r.scalar();
+    }
+    return ResourceLimits(limits);
+  }();
 
-  // Persist quota in memory and add the role into the corresponding
-  // allocation group.
   roles[role].quotaGuarantees =
     ResourceQuantities::fromScalarResources(quota.info.guarantee());
 
@@ -1437,7 +1446,7 @@ void HierarchicalAllocatorProcess::removeQuota(
   CHECK(initialized);
 
   // Do not allow removing quota if it is not set.
-  CHECK(!getGuarantees(role).empty());
+  CHECK(!getGuarantees(role).empty() || !getLimits(role).empty());
 
   Role& r = roles.at(role);
 
@@ -1445,8 +1454,8 @@ void HierarchicalAllocatorProcess::removeQuota(
   LOG(INFO) << "Removed quota " << r.quotaGuarantees
             << " for role '" << role << "'";
 
-  // Remove the role from the quota'ed allocation group.
   r.quotaGuarantees = ResourceQuantities();
+  r.quotaLimits = ResourceLimits();
   if (r.isEmpty()) {
     roles.erase(role);
   }
@@ -1683,7 +1692,7 @@ void HierarchicalAllocatorProcess::__allocate()
   // Currently, only top level roles can have quota set and thus
   // we only track consumed quota for top level roles.
   foreachpair (const string& role, const Role& r, roles) {
-    if (r.quotaGuarantees.empty()) {
+    if (!r.quotaGuarantees.empty() || !r.quotaLimits.empty()) {
       logHeadroomInfo = true;
       // Note, `reservationScalarQuantities` in `struct role`
       // is hierarchical aware, thus it also includes subrole reservations.
@@ -1700,7 +1709,8 @@ void HierarchicalAllocatorProcess::__allocate()
     const string& topLevelRole = strings::contains(role, "/") ?
       role.substr(0, role.find('/')) : role;
 
-    if (getGuarantees(topLevelRole).empty()) {
+    if (getGuarantees(topLevelRole).empty() &&
+        getLimits(topLevelRole).empty()) {
       continue;
     }
 
@@ -2024,7 +2034,7 @@ void HierarchicalAllocatorProcess::__allocate()
 
     foreach (const string& role, roleSorter->sort()) {
       // In the second allocation stage, we only allocate
-      // for non-quota roles.
+      // to roles with default guarantees (i.e. no guarantees).
       if (!getGuarantees(role).empty()) {
         continue;
       }
@@ -2530,6 +2540,13 @@ const ResourceQuantities& 
HierarchicalAllocatorProcess::getGuarantees(
 }
 
 
+const ResourceLimits& HierarchicalAllocatorProcess::getLimits(
+    const string& role) const
+{
+  return roles.contains(role) ? roles.at(role).quotaLimits : 
defaultQuotaLimits;
+}
+
+
 void HierarchicalAllocatorProcess::trackFrameworkUnderRole(
     const FrameworkID& frameworkId,
     const string& role)
diff --git a/src/master/allocator/mesos/hierarchical.hpp 
b/src/master/allocator/mesos/hierarchical.hpp
index e52253d..1a3329d 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -117,15 +117,17 @@ struct Role
   // Note that non-scalar resources, such as ports, are excluded.
   ResourceQuantities reservationScalarQuantities;
 
-  // Configured guaranteed resource quantities for this role.
-  // By default, a role has no guarantee.
+  // Configured guaranteed resource quantities and resource limits for
+  // this role. By default, a role has no guarantee and no limit.
   ResourceQuantities quotaGuarantees;
+  ResourceLimits quotaLimits;
 
   bool isEmpty() const
   {
     return frameworks.empty() &&
            reservationScalarQuantities.empty() &&
-           quotaGuarantees.empty();
+           quotaGuarantees.empty() &&
+           quotaLimits.empty();
   }
 };
 
@@ -617,7 +619,7 @@ protected:
   // Factory function for framework sorters.
   const std::function<Sorter*()> frameworkSorterFactory;
 
-  // By default, roles have empty quota guarantees.
+  // By default, roles have empty quota guarantees and limits.
   //
   // We keep this in memory so that roles that are absent in the `roles` map
   // could also keep their quota state in memory.
@@ -625,6 +627,7 @@ protected:
   // TODO(mzhu): remove this once we have proper role life cycle management
   // such that every role would have an entry in the `roles` map.
   const ResourceQuantities defaultQuotaGuarantees;
+  const ResourceLimits defaultQuotaLimits;
 
 private:
   bool isFrameworkTrackedUnderRole(
@@ -632,6 +635,7 @@ private:
       const std::string& role) const;
 
   const ResourceQuantities& getGuarantees(const std::string& role) const;
+  const ResourceLimits& getLimits(const std::string& role) const;
 
   void trackFrameworkUnderRole(
       const FrameworkID& frameworkId,

Reply via email to