You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2018/03/05 22:53:30 UTC

[1/2] mesos git commit: Updated comment and variables to decouple quota limit and guarantee.

Repository: mesos
Updated Branches:
  refs/heads/master 50d239b74 -> 8f4552e37


Updated comment and variables to decouple quota limit and guarantee.

We currently do not differentiate between quota guarantee and limit
i.e. quota currently is set for both guarantee and limit. This patch
updates comments and related variable names in the allocator to
reflect the difference between quota guarantee and limit.

Also fixed a few typos and removed some outdated comments.

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


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

Branch: refs/heads/master
Commit: e7eaf0d3495867f3db92c76942bfaed9736e60fa
Parents: 50d239b
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Mon Mar 5 13:40:38 2018 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Mar 5 14:52:47 2018 -0800

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 134 ++++++++++++-----------
 src/master/allocator/mesos/hierarchical.hpp |  49 ++++-----
 2 files changed, 88 insertions(+), 95 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e7eaf0d3/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 58aa83f..e0ab836 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1556,12 +1556,13 @@ void HierarchicalAllocatorProcess::__allocate()
   // TODO(vinod): Implement a smarter sorting algorithm.
   std::random_shuffle(slaveIds.begin(), slaveIds.end());
 
-  // Returns the __quantity__ of resources allocated to a quota role. Since we
-  // account for reservations and persistent volumes toward quota, we strip
-  // reservation and persistent volume related information for comparability.
-  // The result is used to determine whether a role's quota is satisfied, and
-  // also to determine how many resources the role would need in order to meet
-  // its quota.
+  // Returns the __quantity__ of resources allocated to a role with
+  // non-default quota. Since we account for reservations and persistent
+  // volumes toward quota, we strip reservation and persistent volumes
+  // related information for comparability. The result is used to
+  // determine whether a role's quota guarantee is satisfied, and
+  // also to determine how many resources the role would need in
+  // order to meet its quota guarantee.
   //
   // NOTE: Revocable resources are excluded in `quotaRoleSorter`.
   auto getQuotaRoleAllocatedResources = [this](const string& role) {
@@ -1574,8 +1575,8 @@ void HierarchicalAllocatorProcess::__allocate()
   };
 
   // We need to keep track of allocated reserved resources for roles
-  // with quota in order to enforce their quota limit. Note these are
-  // __quantities__ with no meta-data.
+  // with a non-default quota in order to enforce their quota.
+  // Note these are __quantities__ with no meta-data.
   hashmap<string, Resources> allocatedReservationScalarQuantities;
 
   // We build the map here to avoid repetitive aggregation
@@ -1615,15 +1616,17 @@ void HierarchicalAllocatorProcess::__allocate()
     }
   }
 
-  // We need to constantly make sure that we are holding back enough unreserved
-  // resources that the remaining quota can later be satisfied when needed:
+  // We need to constantly make sure that we are holding back enough
+  // unreserved resources that the remaining quota guarantee can later
+  // be satisfied when needed:
   //
   //   Required unreserved headroom =
-  //     sum (unsatisfied quota(r) - unallocated reservations(r))
-  //       for each quota role r
+  //     sum (unsatisfied quota guarantee(r) - unallocated reservations(r))
+  //       for each role r
   //
-  // Given the above, if a role has more reservations than quota,
-  // we don't need to hold back any unreserved headroom for it.
+  // Given the above, if a role has more reservations than quota
+  // guarantee, we don't need to hold back any unreserved headroom
+  // for it.
   Resources requiredHeadroom;
   foreachpair (const string& role, const Quota& quota, quotas) {
     // NOTE: Revocable resources are excluded in `quotaRoleSorter`.
@@ -1633,7 +1636,7 @@ void HierarchicalAllocatorProcess::__allocate()
     const Resources guarantee = quota.info.guarantee();
 
     if (allocated.contains(guarantee)) {
-      continue; // Quota already satisifed.
+      continue; // Quota guarantee already satisfied.
     }
 
     Resources unallocated = guarantee - allocated;
@@ -1647,7 +1650,7 @@ void HierarchicalAllocatorProcess::__allocate()
 
   // We will allocate resources while ensuring that the required
   // unreserved non-revocable headroom is still available. Otherwise,
-  // we will not be able to satisfy quota later.
+  // we will not be able to satisfy the quota guarantee later.
   //
   //   available headroom = unallocated unreserved non-revocable resources
   //
@@ -1721,9 +1724,19 @@ void HierarchicalAllocatorProcess::__allocate()
   // allocated in the current cycle.
   hashmap<SlaveID, Resources> offeredSharedResources;
 
-  // Quota comes first and fair share second. Here we process only those
-  // roles for which quota is set (quota'ed roles). Such roles form a
-  // special allocation group with a dedicated sorter.
+  // Quota guarantee comes first and bursting above the quota guarantee
+  // up to the quota limit comes second. Here we process only those
+  // roles for that have a non-empty quota guarantee.
+  //
+  // NOTE: Even though we keep track of the available headroom, we still
+  // dedicate the first stage to satisfy role's quota guarantee. The reason
+  // is that quota guarantee headroom only acts as a quantity guarantee.
+  // Frameworks might have filters or capabilities such that those resources
+  // set aside for the headroom cannot be used by these frameworks, resulting
+  // in unsatisfied quota guarantee (despite enough quota headroom). Thus
+  // we try to satisfy the quota guarantee in this first stage so that those
+  // roles with unsatisfied guarantee can have more choices and higher
+  // probability in getting their guarantee satisfied.
   foreach (const SlaveID& slaveId, slaveIds) {
     foreach (const string& role, quotaRoleSorter->sort()) {
       CHECK(quotas.contains(role));
@@ -1760,11 +1773,8 @@ void HierarchicalAllocatorProcess::__allocate()
           (getQuotaRoleAllocatedResources(role) -
                roleAllocatedReservationScalarQuantities);
 
-      // If quota for the role is considered satisfied, then we only
-      // further allocate reservations for the role.
-      //
       // This is a scalar quantity with no meta-data.
-      Resources unsatisfiedQuota = Resources(quota.info.guarantee()) -
+      Resources unsatisfiedQuotaGuarantee = Resources(quota.info.guarantee()) -
         resourcesChargedAgainstQuota;
 
       // Fetch frameworks according to their fair share.
@@ -1815,19 +1825,19 @@ void HierarchicalAllocatorProcess::__allocate()
           }
         }
 
-        // We allocate the role's reservations as well as any unreserved
-        // resources while ensuring the role stays within its quota limits.
-        // This means that we'll "chop" the unreserved resources up to
-        // the quota limit if necessary.
+        // In this first stage, we allocate the role's reservations as well as
+        // any unreserved resources while ensuring the role stays within its
+        // quota guarantee. This means that we'll "chop" the unreserved
+        // resources up to the quota guarantee if necessary.
         //
         // E.g. A role has no allocations or reservations yet and a 10 cpu
         //      quota limit. We'll chop a 15 cpu agent down to only
-        //      allocate 10 cpus to the role to keep it within its limit.
+        //      allocate 10 cpus to the role to keep it within its guarantee.
         //
         // In the case that the role needs some of the resources on this
         // agent to make progress towards its quota, or the role is being
         // allocated some reservation(s), we'll *also* allocate all of
-        // the resources for which it does not have quota.
+        // the resources for which it does not have quota guarantee.
         //
         // E.g. The agent has 1 cpu, 1024 mem, 1024 disk, 1 gpu, 5 ports
         //      and the role has quota for 1 cpu, 1024 mem. We'll include
@@ -1857,19 +1867,19 @@ void HierarchicalAllocatorProcess::__allocate()
         // These are __quantities__ with no meta-data.
         Resources newQuotaAllocationScalarQuantities;
 
-        // We put resource that this role has no quota for in
-        // `nonQuotaResources` tentatively.
-        Resources nonQuotaResources;
+        // We put resource that this role has no quota guarantee for in
+        // `nonQuotaGuaranteeResources` tentatively.
+        Resources nonQuotaGuaranteeResources;
 
         Resources unreserved = available.nonRevocable().unreserved();
 
-        set<string> quotaResourceNames =
+        set<string> quotaGuaranteeResourceNames =
           Resources(quota.info.guarantee()).names();
 
         // When "chopping" resources, there is more than 1 "chop" that
-        // can be done to satisfy the limits. Consider the case with
+        // can be done to satisfy the guarantee. Consider the case with
         // two disks of 1GB, one is PATH and another is MOUNT. And a
-        // role has a "disk" quota of 1GB. We could pick either of
+        // role has a "disk" quota guarantee of 1GB. We could pick either of
         // the disks here, but not both.
         //
         // In order to avoid repeatedly choosing the same "chop" of
@@ -1882,14 +1892,13 @@ void HierarchicalAllocatorProcess::__allocate()
         foreach (Resource& resource, resourceVector) {
           if (resource.type() != Value::SCALAR) {
             // We currently do not support quota for non-scalar resources,
-            // add it to `nonQuotaResources`. See `nonQuotaResources`
-            // regarding how these resources are allocated.
-            nonQuotaResources += resource;
+            // add it to `nonQuotaGuaranteeResources`.
+            nonQuotaGuaranteeResources += resource;
             continue;
           }
 
-          if (quotaResourceNames.count(resource.name()) == 0) {
-            // Allocating resource that this role has NO quota for,
+          if (quotaGuaranteeResourceNames.count(resource.name()) == 0) {
+            // Allocating resource that this role has NO quota guarantee for,
             // the limit concern here is that it should not break the
             // quota headroom.
             //
@@ -1898,7 +1907,7 @@ void HierarchicalAllocatorProcess::__allocate()
             Resources upperLimitScalarQuantities =
               availableHeadroom - requiredHeadroom -
               (newQuotaAllocationScalarQuantities +
-                nonQuotaResources.createStrippedScalarQuantity());
+                nonQuotaGuaranteeResources.createStrippedScalarQuantity());
 
             Option<Value::Scalar> limitScalar =
               upperLimitScalarQuantities.get<Value::Scalar>(resource.name());
@@ -1908,20 +1917,21 @@ void HierarchicalAllocatorProcess::__allocate()
             }
 
             if (Resources::shrink(&resource, limitScalar.get())) {
-              nonQuotaResources += resource;
+              nonQuotaGuaranteeResources += resource;
             }
           } else {
-            // Allocating resource that this role has quota for,
+            // Allocating resource that this role has quota guarantee for,
             // the limit concern is that it should not exceed this
-            // role's unsatisfied quota.
+            // role's unsatisfied quota guarantee.
             Resources upperLimitScalarQuantities =
-              unsatisfiedQuota - newQuotaAllocationScalarQuantities;
+              unsatisfiedQuotaGuarantee -
+                newQuotaAllocationScalarQuantities;
 
             Option<Value::Scalar> limitScalar =
               upperLimitScalarQuantities.get<Value::Scalar>(resource.name());
 
             if (limitScalar.isNone()) {
-              continue; // Quota limit already met.
+              continue; // Quota guarantee already met.
             }
 
             if (Resources::shrink(&resource, limitScalar.get())) {
@@ -1932,12 +1942,11 @@ void HierarchicalAllocatorProcess::__allocate()
           }
         }
 
-        // We include the non-quota resources (with headroom taken
+        // We include the non-quota guarantee resources (with headroom taken
         // into account) if this role is being allocated some resources
-        // already: either some quota resources or a reservation
-        // (possibly with quota resources).
+        // already: either some quota guarantee resources or a reservation.
         if (!resources.empty()) {
-          resources += nonQuotaResources;
+          resources += nonQuotaGuaranteeResources;
         }
 
         // It is safe to break here, because all frameworks under a role would
@@ -1968,8 +1977,7 @@ void HierarchicalAllocatorProcess::__allocate()
           });
         }
 
-        // If the framework filters these resources, ignore. The unallocated
-        // part of the quota will not be allocated to other roles.
+        // If the framework filters these resources, ignore.
         if (isFiltered(frameworkId, role, slaveId, resources)) {
           continue;
         }
@@ -1980,15 +1988,13 @@ void HierarchicalAllocatorProcess::__allocate()
 
         resources.allocate(role);
 
-        // NOTE: We perform "coarse-grained" allocation for quota'ed
-        // resources, which may lead to overcommitment of resources beyond
-        // quota. This is fine since quota currently represents a guarantee.
         offerable[frameworkId][role][slaveId] += resources;
         offeredSharedResources[slaveId] += resources.shared();
 
-        unsatisfiedQuota -= newQuotaAllocationScalarQuantities;
+        unsatisfiedQuotaGuarantee -=
+          newQuotaAllocationScalarQuantities;
 
-        // Track quota headroom change.
+        // Track quota guarantee headroom change.
         requiredHeadroom -= newQuotaAllocationScalarQuantities;
         availableHeadroom -=
           resources.unreserved().createStrippedScalarQuantity();
@@ -2016,11 +2022,12 @@ void HierarchicalAllocatorProcess::__allocate()
   }
 
   // Similar to the first stage, we will allocate resources while ensuring
-  // that the required unreserved non-revocable headroom is still available.
-  // Otherwise, we will not be able to satisfy quota later. Reservations to
-  // non-quota roles and revocable resources will always be included
-  // in the offers since these are not part of the headroom (and
-  // therefore can't be used to satisfy quota).
+  // that the required unreserved non-revocable headroom is still available
+  // for unsastified quota guarantees. Otherwise, we will not be able to
+  // satisfy quota guarantees later. Reservations to non-quota roles and
+  // revocable resources will always be included in the offers since these
+  // are not part of the headroom (and therefore can't be used to satisfy
+  // quota guarantees).
 
   foreach (const SlaveID& slaveId, slaveIds) {
     foreach (const string& role, roleSorter->sort()) {
@@ -2152,9 +2159,6 @@ void HierarchicalAllocatorProcess::__allocate()
 
         // NOTE: We perform "coarse-grained" allocation, meaning that we always
         // allocate the entire remaining slave resources to a single framework.
-        //
-        // NOTE: We may have already allocated some resources on the current
-        // agent as part of quota.
         offerable[frameworkId][role][slaveId] += resources;
         offeredSharedResources[slaveId] += resources.shared();
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/e7eaf0d3/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 37d2df5..98285d2 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -444,19 +444,13 @@ protected:
   // (e.g. some tasks and/or executors are consuming resources under the role).
   hashmap<std::string, hashset<FrameworkID>> roles;
 
-  // Configured quota for each role, if any. Setting quota for a role
-  // changes the order that the role's frameworks are offered
-  // resources. Quota comes before fair share, hence setting quota moves
-  // the role's frameworks towards the front of the allocation queue.
-  //
-  // NOTE: We currently associate quota with roles, but this may
-  // change in the future.
+  // Configured quota for each role, if any. If a role does not have
+  // an entry here it has the default quota of (no guarantee, no limit).
   hashmap<std::string, Quota> quotas;
 
   // Aggregated resource reservations on all agents tied to a
   // particular role, if any. These are stripped scalar quantities
-  // that contain no meta-data. Used for accounting resource
-  // reservations for quota limit.
+  // that contain no meta-data.
   //
   // Only roles with non-empty reservations will be stored in the map.
   hashmap<std::string, Resources> reservationScalarQuantities;
@@ -473,11 +467,14 @@ protected:
   // The master's domain, if any.
   Option<DomainInfo> domain;
 
-  // There are two stages of allocation. During the first stage resources
-  // are allocated only to frameworks under roles with quota set. During
-  // the second stage remaining resources that would not be required to
-  // satisfy un-allocated quota are then allocated to frameworks under
-  // roles with no quota set.
+  // There are two stages of allocation:
+  //
+  //   Stage 1: Allocate to satisfy quota guarantees.
+  //
+  //   Stage 2: Allocate above quota guarantees up to quota limits.
+  //            Note that we need to hold back enough "headroom"
+  //            to ensure that any unsatisfied quota can be
+  //            satisfied later.
   //
   // Each stage comprises two levels of sorting, hence "hierarchical".
   // Level 1 sorts across roles:
@@ -510,24 +507,16 @@ protected:
   // The total cluster resources are used as the resource pool.
   process::Owned<Sorter> roleSorter;
 
-  // A dedicated sorter for roles for which quota is set. This sorter
-  // determines the order in which quota'ed roles are allocated resources
-  // during Level 1 of the first stage. Quota'ed roles have resources
-  // allocated up to their allocated quota (the first stage) prior to
-  // non-quota'ed roles (the second stage). Since only non-revocable
+  // A dedicated sorter for roles that have a non-default quota.
+  // This sorter determines the order in which guarantees are allocated
+  // during Level 1 of the first stage. Since only non-revocable
   // resources are available for quota, the total cluster non-revocable
-  // resoures are used as the resource pool.
-  //
-  // NOTE: A role appears in `quotaRoleSorter` if it has a quota (even if
-  // no frameworks are currently registered in that role). In contrast,
-  // `roleSorter` only contains entries for roles with one or more
-  // registered frameworks.
+  // resources are used as the resource pool.
   //
-  // NOTE: We do not include revocable resources in the quota role sorter,
-  // because the quota role sorter's job is to perform fair sharing between
-  // the quota roles as it pertains to their level of quota satisfaction.
-  // Since revocable resources do not increase a role's level of satisfaction
-  // toward its quota, we choose to exclude them from the quota role sorter.
+  // NOTE: A role appears in `quotaRoleSorter` if it has a non-default
+  // quota (even if no frameworks are currently registered in that role).
+  // In contrast, `roleSorter` only contains entries for roles with one
+  // or more registered frameworks.
   process::Owned<Sorter> quotaRoleSorter;
 
   // A collection of sorters, one per active role. Each sorter determines


[2/2] mesos git commit: Added a TODO to clean up the quota sorter tech debt.

Posted by bm...@apache.org.
Added a TODO to clean up the quota sorter tech debt.


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

Branch: refs/heads/master
Commit: 8f4552e37a76b3eee6affa34729b29582eae4635
Parents: e7eaf0d
Author: Benjamin Mahler <bm...@apache.org>
Authored: Mon Mar 5 14:53:04 2018 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Mar 5 14:53:04 2018 -0800

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.hpp | 7 +++++++
 1 file changed, 7 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/8f4552e3/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 98285d2..955ae3e 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -507,6 +507,13 @@ protected:
   // The total cluster resources are used as the resource pool.
   process::Owned<Sorter> roleSorter;
 
+  // TODO(bmahler): Remove this in favor of either using the same sorting
+  // between satisfying guarantees and bursting above guarantees up to
+  // limits, or have a different sorting technique specifically for
+  // satisfying guarantees (e.g. MESOS-8026). This is tech debt from
+  // when a "quota role" was considered different from a "non-quota"
+  // role. However, they are the same, one just has a default quota.
+  //
   // A dedicated sorter for roles that have a non-default quota.
   // This sorter determines the order in which guarantees are allocated
   // during Level 1 of the first stage. Since only non-revocable