Updated the allocator to untrack allocations via a single code path. Review: https://reviews.apache.org/r/64357/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/69b3aa0c Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/69b3aa0c Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/69b3aa0c Branch: refs/heads/1.4.x Commit: 69b3aa0cc60f29e632d0194853a54ab37b6cd42a Parents: cd450bc Author: Meng Zhu <[email protected]> Authored: Thu Dec 7 13:14:10 2017 -0800 Committer: Benjamin Mahler <[email protected]> Committed: Wed May 2 16:43:29 2018 -0700 ---------------------------------------------------------------------- src/master/allocator/mesos/hierarchical.cpp | 58 ++++++++++++++++-------- src/master/allocator/mesos/hierarchical.hpp | 12 ++++- 2 files changed, 51 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/69b3aa0c/src/master/allocator/mesos/hierarchical.cpp ---------------------------------------------------------------------- diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp index 640f9ce..b6db12e 100644 --- a/src/master/allocator/mesos/hierarchical.cpp +++ b/src/master/allocator/mesos/hierarchical.cpp @@ -313,6 +313,8 @@ void HierarchicalAllocatorProcess::removeFramework( foreach (const string& role, framework.roles) { // Might not be in 'frameworkSorters[role]' because it // was previously deactivated and never re-added. + // + // TODO(mzhu): This check may no longer be necessary. if (!frameworkSorters.contains(role) || !frameworkSorters.at(role)->contains(frameworkId.value())) { continue; @@ -325,14 +327,7 @@ void HierarchicalAllocatorProcess::removeFramework( foreachpair (const SlaveID& slaveId, const Resources& allocated, allocation) { - roleSorter->unallocated(role, slaveId, allocated); - frameworkSorters.at(role)->remove(slaveId, allocated); - - if (quotas.contains(role)) { - // See comment at `quotaRoleSorter` declaration - // regarding non-revocable. - quotaRoleSorter->unallocated(role, slaveId, allocated.nonRevocable()); - } + untrackAllocatedResources(slaveId, frameworkId, allocated); } untrackFrameworkUnderRole(frameworkId, role); @@ -1098,16 +1093,7 @@ void HierarchicalAllocatorProcess::recoverResources( const Owned<Sorter>& frameworkSorter = frameworkSorters.at(role); if (frameworkSorter->contains(frameworkId.value())) { - frameworkSorter->unallocated(frameworkId.value(), slaveId, resources); - frameworkSorter->remove(slaveId, resources); - roleSorter->unallocated(role, slaveId, resources); - - if (quotas.contains(role)) { - // See comment at `quotaRoleSorter` declaration - // regarding non-revocable - quotaRoleSorter->unallocated( - role, slaveId, resources.nonRevocable()); - } + untrackAllocatedResources(slaveId, frameworkId, resources); // Stop tracking the framework under this role if it's no longer // subscribed and no longer has resources allocated to the role. @@ -2419,6 +2405,42 @@ void HierarchicalAllocatorProcess::trackAllocatedResources( } } + +void HierarchicalAllocatorProcess::untrackAllocatedResources( + const SlaveID& slaveId, + const FrameworkID& frameworkId, + const Resources& allocated) +{ + // TODO(mzhu): Add a `CHECK(slaves.contains(slaveId));` + // here once MESOS-621 is resolved. Ideally, `removeSlave()` + // should unallocate resources in the framework sorters. + // But currently, a slave is removed first via `removeSlave()` + // and later a call to `recoverResources()` occurs to recover + // the framework's resources. + CHECK(frameworks.contains(frameworkId)); + + // TODO(bmahler): Calling allocations() is expensive since it has + // to construct a map. Avoid this. + foreachpair (const string& role, + const Resources& allocation, + allocated.allocations()) { + CHECK(roleSorter->contains(role)); + CHECK(frameworkSorters.contains(role)); + CHECK(frameworkSorters.at(role)->contains(frameworkId.value())); + + frameworkSorters.at(role)->unallocated( + frameworkId.value(), slaveId, allocation); + frameworkSorters.at(role)->remove(slaveId, allocation); + + roleSorter->unallocated(role, slaveId, allocation); + + if (quotas.contains(role)) { + // See comment at `quotaRoleSorter` declaration regarding non-revocable. + quotaRoleSorter->unallocated(role, slaveId, allocation.nonRevocable()); + } + } +} + } // namespace internal { } // namespace allocator { } // namespace master { http://git-wip-us.apache.org/repos/asf/mesos/blob/69b3aa0c/src/master/allocator/mesos/hierarchical.hpp ---------------------------------------------------------------------- diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp index a53a952..0d30179 100644 --- a/src/master/allocator/mesos/hierarchical.hpp +++ b/src/master/allocator/mesos/hierarchical.hpp @@ -539,11 +539,21 @@ private: // the agent and the master are both configured with a fault domain. bool isRemoteSlave(const Slave& slave) const; - // Helper to track used resources on an agent. + // Helper to track allocated resources on an agent. void trackAllocatedResources( const SlaveID& slaveId, const FrameworkID& frameworkId, const Resources& allocated); + + // Helper to untrack resources that are no longer allocated on an agent. + void untrackAllocatedResources( + const SlaveID& slaveId, + const FrameworkID& frameworkId, + const Resources& allocated); + + // Helper that removes all existing offer filters for the given slave + // id. + void removeFilters(const SlaveID& slaveId); };
