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 2017/12/19 05:40:55 UTC
[2/3] mesos git commit: Fixed a bug where insufficient headroom is
held for quota.
Fixed a bug where insufficient headroom is held for quota.
If a role has more reservation than its quota, the current quota
headroom calculation is insufficient in guaranteeing quota
allocation. See MESOS-8339.
This patch fixes the issue by computing the amount of unreserved
resources we need to hold back such that each quota role can have
its quota satisfied. Previously, we included the quota role
unallocated reservations as part of the headroom since we knew it
would be held back, but we didn't handle the case that a quota role
had more reservations than quota.
Review: https://reviews.apache.org/r/64698/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c9933c5a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c9933c5a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c9933c5a
Branch: refs/heads/master
Commit: c9933c5a5efc2202bd2f346f2b0861aa02853922
Parents: 454e7f2
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Mon Dec 18 19:40:59 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Dec 18 21:15:30 2017 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 79 +++++++++++++-----------
1 file changed, 43 insertions(+), 36 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/c9933c5a/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 2cabafe..1c767f7 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1821,41 +1821,51 @@ void HierarchicalAllocatorProcess::__allocate()
// Frameworks in a quota'ed role may temporarily reject resources by
// filtering or suppressing offers. Hence, quotas may not be fully
- // allocated and if so we need to hold back some headroom to ensure
- // the quota can later be satisfied when needed.
+ // allocated and if so we need to hold back enough unreserved
+ // resources to ensure the quota can later be satisfied when needed:
+ //
+ // Required unreserved headroom =
+ // sum (unsatisfied quota(r) - unallocated reservations(r))
+ // for each quota 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.
Resources requiredHeadroom;
foreachpair (const string& role, const Quota& quota, quotas) {
- // Compute the amount of quota that the role does not have allocated.
- //
// NOTE: Revocable resources are excluded in `quotaRoleSorter`.
// NOTE: Only scalars are considered for quota.
+ // NOTE: The following should all be quantities with no meta-data!
Resources allocated = getQuotaRoleAllocatedResources(role);
- const Resources required = quota.info.guarantee();
- requiredHeadroom += (required - allocated);
+ const Resources guarantee = quota.info.guarantee();
+
+ if (allocated.contains(guarantee)) {
+ continue; // Quota already satisifed.
+ }
+
+ Resources unallocated = guarantee - allocated;
+
+ Resources unallocatedReservations =
+ reservationScalarQuantities.get(role).getOrElse(Resources()) -
+ allocatedReservationScalarQuantities.get(role).getOrElse(Resources());
+
+ requiredHeadroom += unallocated - unallocatedReservations;
}
- // The amount of headroom currently available consists of
- // unallocated resources that can be allocated to satisfy quota.
- // What that means is that the resources need to be:
- //
- // (1) Non-revocable
- // (2) Unreserved, or reserved to a quota role.
+ // 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).
//
- // TODO(bmahler): Note that we currently do not consider quota
- // headroom on an per-role basis, which means we may not hold
- // back sufficient headroom for a particular quota role if some
- // other quota roles have more reservations than quota.
- // See MESOS-8339.
+ // available headroom = unallocated unreserved non-revocable resources
//
- // We will allocate resources while ensuring that the required
- // 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 available headroom for quota.
+ // We compute this as:
//
- // available headroom = unallocated resources -
- // unallocated non-quota role reservations -
- // unallocated revocable resources
+ // available headroom = total resources -
+ // allocated resources -
+ // unallocated reservations -
+ // unallocated revocable resources
// NOTE: `totalScalarQuantities` omits dynamic reservation,
// persistent volume info, and allocation info. We additionally
@@ -1872,31 +1882,28 @@ void HierarchicalAllocatorProcess::__allocate()
roleSorter->allocationScalarQuantities(role).toUnreserved();
}
- // Subtract non quota role reservations.
+ // Subtract all unallocated reservations.
foreachkey (const string& role, reservationScalarQuantities) {
- if (quotas.contains(role)) {
- continue; // Skip quota roles.
- }
-
hashmap<SlaveID, Resources> allocations;
- if (roleSorter->contains(role)) {
+ if (quotaRoleSorter->contains(role)) {
+ allocations = quotaRoleSorter->allocation(role);
+ } else if (roleSorter->contains(role)) {
allocations = roleSorter->allocation(role);
}
- Resources unallocatedNonQuotaRoleReservations =
+ Resources unallocatedReservations =
reservationScalarQuantities.get(role).getOrElse(Resources());
foreachvalue (const Resources& resources, allocations) {
// NOTE: `totalScalarQuantities` omits dynamic reservation,
// persistent volume info, and allocation info. We additionally
// remove the static reservations here via `toUnreserved()`.
- unallocatedNonQuotaRoleReservations -=
+ unallocatedReservations -=
resources.reserved().createStrippedScalarQuantity().toUnreserved();
}
- // Subtract the unallocated reservations for this non quota
- // role from the headroom.
- availableHeadroom -= unallocatedNonQuotaRoleReservations;
+ // Subtract the unallocated reservations for this role from the headroom.
+ availableHeadroom -= unallocatedReservations;
}
// Subtract revocable resources.