Tracked resource reservations in the allocator. Resource reservations need to be tracked to make sure quota limit will not be exceeded in the presence of resource reservations.
Review: https://reviews.apache.org/r/64303/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/35b0e1cf Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/35b0e1cf Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/35b0e1cf Branch: refs/heads/1.4.x Commit: 35b0e1cf3da823ca5154989a283e06a37ba5e462 Parents: 69b3aa0 Author: Meng Zhu <[email protected]> Authored: Mon Dec 11 16:45:49 2017 -0800 Committer: Benjamin Mahler <[email protected]> Committed: Wed May 2 16:43:35 2018 -0700 ---------------------------------------------------------------------- src/master/allocator/mesos/hierarchical.cpp | 48 ++++++++++++++++++++++++ src/master/allocator/mesos/hierarchical.hpp | 28 ++++++++++++++ 2 files changed, 76 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/35b0e1cf/src/master/allocator/mesos/hierarchical.cpp ---------------------------------------------------------------------- diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp index b6db12e..b70868d 100644 --- a/src/master/allocator/mesos/hierarchical.cpp +++ b/src/master/allocator/mesos/hierarchical.cpp @@ -518,6 +518,8 @@ void HierarchicalAllocatorProcess::addSlave( slave.maintenance = Slave::Maintenance(unavailability.get()); } + trackReservations(total.reservations()); + roleSorter->add(slaveId, total); // See comment at `quotaRoleSorter` declaration regarding non-revocable. @@ -590,6 +592,8 @@ void HierarchicalAllocatorProcess::removeSlave( // See comment at `quotaRoleSorter` declaration regarding non-revocable. quotaRoleSorter->remove(slaveId, slaves.at(slaveId).total.nonRevocable()); + untrackReservations(slaves.at(slaveId).total.reservations()); + slaves.erase(slaveId); allocationCandidates.erase(slaveId); @@ -2303,6 +2307,42 @@ void HierarchicalAllocatorProcess::untrackFrameworkUnderRole( } +void HierarchicalAllocatorProcess::trackReservations( + const hashmap<std::string, Resources>& reservations) +{ + foreachpair (const string& role, + const Resources& resources, reservations) { + // We remove the static reservation metadata here via `toUnreserved()`. + const Resources scalarQuantitesToTrack = + resources.createStrippedScalarQuantity().toUnreserved(); + + reservationScalarQuantities[role] += scalarQuantitesToTrack; + } +} + + +void HierarchicalAllocatorProcess::untrackReservations( + const hashmap<std::string, Resources>& reservations) +{ + foreachpair (const string& role, + const Resources& resources, reservations) { + CHECK(reservationScalarQuantities.contains(role)); + Resources& currentReservationQuantity = + reservationScalarQuantities.at(role); + + // We remove the static reservation metadata here via `toUnreserved()`. + const Resources scalarQuantitesToUntrack = + resources.createStrippedScalarQuantity().toUnreserved(); + CHECK(currentReservationQuantity.contains(scalarQuantitesToUntrack)); + currentReservationQuantity -= scalarQuantitesToUntrack; + + if (currentReservationQuantity.empty()) { + reservationScalarQuantities.erase(role); + } + } +} + + bool HierarchicalAllocatorProcess::updateSlaveTotal( const SlaveID& slaveId, const Resources& total) @@ -2319,6 +2359,14 @@ bool HierarchicalAllocatorProcess::updateSlaveTotal( slave.total = total; + hashmap<std::string, Resources> oldReservations = oldTotal.reservations(); + hashmap<std::string, Resources> newReservations = total.reservations(); + + if (oldReservations != newReservations) { + untrackReservations(oldReservations); + trackReservations(newReservations); + } + // Currently `roleSorter` and `quotaRoleSorter`, being the root-level // sorters, maintain all of `slaves[slaveId].total` (or the `nonRevocable()` // portion in the case of `quotaRoleSorter`) in their own totals (which http://git-wip-us.apache.org/repos/asf/mesos/blob/35b0e1cf/src/master/allocator/mesos/hierarchical.hpp ---------------------------------------------------------------------- diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp index 0d30179..3f15b2c 100644 --- a/src/master/allocator/mesos/hierarchical.hpp +++ b/src/master/allocator/mesos/hierarchical.hpp @@ -442,6 +442,14 @@ protected: // change in the future. 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. + // + // Only roles with non-empty reservations will be stored in the map. + hashmap<std::string, Resources> reservationScalarQuantities; + // Slaves to send offers for. Option<hashset<std::string>> whitelist; @@ -529,6 +537,26 @@ private: const FrameworkID& frameworkId, const std::string& role); + // `trackReservations` and `untrackReservations` are helpers + // to track role resource reservations. We need to keep + // track of reservations to enforce role quota limit + // in the presence of unallocated reservations. See MESOS-4527. + // + // TODO(mzhu): Ideally, we want these helpers to instead track the + // reservations as *allocated* in the sorters even when the + // reservations have not been allocated yet. This will help to: + // + // (1) Solve the fairness issue when roles with unallocated + // reservations may game the allocator (See MESOS-8299). + // + // (2) Simplify the quota enforcement logic -- the allocator + // would no longer need to track reservations separately. + void trackReservations( + const hashmap<std::string, Resources>& reservations); + + void untrackReservations( + const hashmap<std::string, Resources>& reservations); + // Helper to update the agent's total resources maintained in the allocator // and the role and quota sorters (whose total resources match the agent's // total resources). Returns true iff the stored agent total was changed.
