This is an automated email from the ASF dual-hosted git repository. mzhu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 43bbe365db469d5e641d71f5884bd0fb1c012ea1 Author: Meng Zhu <[email protected]> AuthorDate: Mon Sep 23 14:51:12 2019 -0700 Refactored `removeFramework()` in the allocator. Now allocated or offered resources are tracked in the slave struct, we no longer need to look up states in the sorter when removing frameworks. This patch makes the framework removal logic more simpler by doing this. This also paves the way to deprecate the framework sorter. Review: https://reviews.apache.org/r/71547 --- src/master/allocator/mesos/hierarchical.cpp | 53 +++++++++++++---------------- src/master/allocator/mesos/hierarchical.hpp | 7 ++-- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp index 467908c..21010de 100644 --- a/src/master/allocator/mesos/hierarchical.cpp +++ b/src/master/allocator/mesos/hierarchical.cpp @@ -712,43 +712,38 @@ void HierarchicalAllocatorProcess::removeFramework( { CHECK(initialized); - // Free up resources on agents if any. + // To free up offered or allocated resources of a framework, we need to + // do two things: update available resources in the agent and update + // tracking info in the role tree and role sorter. + // We do both at the same time. foreachvalue (Slave& slave, slaves) { - slave.increaseAvailable( - frameworkId, - slave.getOfferedOrAllocated().get(frameworkId).getOrElse(Resources())); - } - - // Update tracking in the role tree and sorters. - - Framework& framework = *CHECK_NOTNONE(getFramework(frameworkId)); - - 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. - Option<Sorter*> frameworkSorter = getFrameworkSorter(role); + const hashmap<FrameworkID, Resources>& offeredOrAllocated = + slave.getOfferedOrAllocated(); + auto frameworkResources = offeredOrAllocated.find(frameworkId); - if (frameworkSorter.isNone() || - !(*frameworkSorter)->contains(frameworkId.value())) { + if (frameworkResources == offeredOrAllocated.end()) { continue; } - hashmap<SlaveID, Resources> allocation = - (*frameworkSorter)->allocation(frameworkId.value()); + VLOG(1) << "Recovering " << frameworkResources->second + << " from removing framework " << frameworkId + << " (agent total: " << slave.getTotal() << "," + << " offered or allocated: " + << slave.getTotalOfferedOrAllocated() << ")"; - // Update the allocation for this framework. - foreachpair (const SlaveID& slaveId, - const Resources& allocated, - allocation) { - untrackAllocatedResources(slaveId, frameworkId, allocated); - } + untrackAllocatedResources( + slave.id, frameworkId, frameworkResources->second); + + // Note: this method might mutate `offeredOrAllocated`. + slave.increaseAvailable(frameworkId, frameworkResources->second); + } + Framework& framework = *CHECK_NOTNONE(getFramework(frameworkId)); + + // Untack framework from roles. + foreach (const string& role, framework.roles) { CHECK(tryUntrackFrameworkUnderRole(framework, role)) - << " Framework: " << frameworkId - << " role: " << role - << " allocation: " << allocation; + << " Framework: " << frameworkId << " role: " << role; } // Transfer ownership of this framework's metrics to diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp index c2f5378..9d0fbe7 100644 --- a/src/master/allocator/mesos/hierarchical.hpp +++ b/src/master/allocator/mesos/hierarchical.hpp @@ -353,6 +353,11 @@ public: return; } + // It is possible that the reference of `offeredOrAllocated_` + // points to the same object as `resources` below. We must + // do subtraction here before any mutation on the object. + totalOfferedOrAllocated -= offeredOrAllocated_; + Resources& resources = offeredOrAllocated.at(frameworkId); CHECK_CONTAINS(resources, offeredOrAllocated_); resources -= offeredOrAllocated_; @@ -360,8 +365,6 @@ public: offeredOrAllocated.erase(frameworkId); } - totalOfferedOrAllocated -= offeredOrAllocated_; - updateAvailable(); }
