Changed allocator to skip allocation on weight and quota changes. Changing weight or quota will no longer trigger a batch allocation. Since the allocator does not rebalance currently offered resources to reflect changes to weight or quota, doing a batch allocation is not useful; instead, the updated quota/weight values will be reflected on the next allocation.
Review: https://reviews.apache.org/r/57788 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d93f2246 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d93f2246 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d93f2246 Branch: refs/heads/master Commit: d93f2246b20f8ee12194624b947b72604e3815a9 Parents: d0f0f9d Author: Neil Conway <[email protected]> Authored: Mon Mar 20 11:07:23 2017 -0700 Committer: Neil Conway <[email protected]> Committed: Wed Apr 26 14:01:48 2017 -0400 ---------------------------------------------------------------------- src/master/allocator/mesos/hierarchical.cpp | 33 +++++++++++++----------- src/tests/hierarchical_allocator_tests.cpp | 13 +++++----- 2 files changed, 25 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/d93f2246/src/master/allocator/mesos/hierarchical.cpp ---------------------------------------------------------------------- diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp index 051f749..57a5e82 100644 --- a/src/master/allocator/mesos/hierarchical.cpp +++ b/src/master/allocator/mesos/hierarchical.cpp @@ -1294,7 +1294,12 @@ void HierarchicalAllocatorProcess::setQuota( LOG(INFO) << "Set quota " << quota.info.guarantee() << " for role '" << role << "'"; - allocate(); + // NOTE: Since quota changes do not result in rebalancing of + // offered resources, we do not trigger an allocation here; the + // quota change will be reflected in subsequent allocations. + // + // If we add the ability for quota changes to incur a rebalancing + // of offered resources, then we should trigger that here. } @@ -1317,7 +1322,12 @@ void HierarchicalAllocatorProcess::removeQuota( metrics.removeQuota(role); - allocate(); + // NOTE: Since quota changes do not result in rebalancing of + // offered resources, we do not trigger an allocation here; the + // quota change will be reflected in subsequent allocations. + // + // If we add the ability for quota changes to incur a rebalancing + // of offered resources, then we should trigger that here. } @@ -1326,33 +1336,26 @@ void HierarchicalAllocatorProcess::updateWeights( { CHECK(initialized); - bool rebalance = false; - // Update the weight for each specified role. foreach (const WeightInfo& weightInfo, weightInfos) { CHECK(weightInfo.has_role()); weights[weightInfo.role()] = weightInfo.weight(); - // The allocator only needs to rebalance if there is a framework - // registered with this role. The roleSorter contains only roles - // for registered frameworks, but quotaRoleSorter contains any role - // with quota set, regardless of whether any frameworks are registered - // with that role. if (quotas.contains(weightInfo.role())) { quotaRoleSorter->update(weightInfo.role(), weightInfo.weight()); } if (roleSorter->contains(weightInfo.role())) { - rebalance = true; roleSorter->update(weightInfo.role(), weightInfo.weight()); } } - // If at least one of the updated roles has registered - // frameworks, then trigger the allocation. - if (rebalance) { - allocate(); - } + // NOTE: Since weight changes do not result in rebalancing of + // offered resources, we do not trigger an allocation here; the + // weight change will be reflected in subsequent allocations. + // + // If we add the ability for weight changes to incur a rebalancing + // of offered resources, then we should trigger that here. } http://git-wip-us.apache.org/repos/asf/mesos/blob/d93f2246/src/tests/hierarchical_allocator_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp index e2cd66d..1e2eb96 100644 --- a/src/tests/hierarchical_allocator_tests.cpp +++ b/src/tests/hierarchical_allocator_tests.cpp @@ -2745,6 +2745,9 @@ TEST_F(HierarchicalAllocatorTest, QuotaAgainstStarvation) // Since `QUOTA_ROLE` is under quota, `agent2`'s resources will // be allocated to `framework1`. + // Trigger the next batch allocation. + Clock::advance(flags.allocation_interval); + expected = Allocation( framework1.id(), {{QUOTA_ROLE, {{agent2.id(), agent2.resources()}}}}); @@ -3974,8 +3977,8 @@ TEST_F(HierarchicalAllocatorTest, UpdateWeight) weightInfos.push_back(createWeightInfo({"role2"}, 2.0)); allocator->updateWeights(weightInfos); - // 'updateWeights' will trigger the allocation immediately, so it does not - // need to manually advance the clock here. + // Advance the clock and trigger a batch allocation. + Clock::advance(flags.allocation_interval); // role1 share = 0.33 (cpus=4, mem=2048) // framework1 share = 1 @@ -4015,15 +4018,13 @@ TEST_F(HierarchicalAllocatorTest, UpdateWeight) weightInfos.push_back(createWeightInfo("role3", 3.0)); allocator->updateWeights(weightInfos); - // 'updateWeights' will not trigger the allocation immediately because no - // framework exists in 'role3' yet. + // 'updateWeights' does not trigger an allocation. // Framework3 registers with 'role3'. FrameworkInfo framework3 = createFrameworkInfo({"role3"}); allocator->addFramework(framework3.id(), framework3, {}, true); - // 'addFramework' will trigger the allocation immediately, so it does not - // need to manually advance the clock here. + // 'addFramework' will trigger an allocation. // role1 share = 0.166 (cpus=2, mem=1024) // framework1 share = 1
