Fixed a bug where quota headroom is under-calculated. When calculating the quota headroom, we failed to consider ancestor's reservation allocated to the child. This leads to under-calculation of available headroom and excessive resources being set aside for headroom. See MESOS-8604.
This patches fixes this issue by counting ancestor's reservation allocated to the child as allocated-reservation even though the child itself has no reservation. Review: https://reviews.apache.org/r/65806/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/740be44e Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/740be44e Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/740be44e Branch: refs/heads/1.4.x Commit: 740be44e7960711acbfc1933035b2f89eabef01a Parents: d592914 Author: Meng Zhu <[email protected]> Authored: Mon Feb 26 19:25:09 2018 -0800 Committer: Benjamin Mahler <[email protected]> Committed: Wed May 2 16:44:26 2018 -0700 ---------------------------------------------------------------------- src/master/allocator/mesos/hierarchical.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/740be44e/src/master/allocator/mesos/hierarchical.cpp ---------------------------------------------------------------------- diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp index afbc94b..e871999 100644 --- a/src/master/allocator/mesos/hierarchical.cpp +++ b/src/master/allocator/mesos/hierarchical.cpp @@ -1594,30 +1594,34 @@ void HierarchicalAllocatorProcess::__allocate() roleSorter->allocationScalarQuantities(role).toUnreserved(); } - // Subtract all unallocated reservations. - foreachkey (const string& role, reservationScalarQuantities) { + // Calculate total allocated reservations. Note that we need to ensure + // we count a reservation for "a" being allocated to "a/b", therefore + // we cannot simply loop over the reservations' roles. + Resources totalAllocatedReservationScalarQuantities; + foreachkey (const string& role, roles) { hashmap<SlaveID, Resources> allocations; if (quotaRoleSorter->contains(role)) { allocations = quotaRoleSorter->allocation(role); } else if (roleSorter->contains(role)) { allocations = roleSorter->allocation(role); + } else { + continue; // This role has no allocation. } - 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()`. - unallocatedReservations -= + totalAllocatedReservationScalarQuantities += resources.reserved().createStrippedScalarQuantity().toUnreserved(); } - - // Subtract the unallocated reservations for this role from the headroom. - availableHeadroom -= unallocatedReservations; } + // Subtract total unallocated reservations. + availableHeadroom -= + Resources::sum(reservationScalarQuantities) - + totalAllocatedReservationScalarQuantities; + // Subtract revocable resources. foreachvalue (const Slave& slave, slaves) { // NOTE: `totalScalarQuantities` omits dynamic reservation,
