You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2017/02/06 01:40:45 UTC
mesos git commit: Prevent gaming of the quota system due to
demand:quota ratio mismatch.
Repository: mesos
Updated Branches:
refs/heads/master 4dbbf44ed -> 34de30cf6
Prevent gaming of the quota system due to demand:quota ratio mismatch.
It is currently possible to game the quota system since the allocator
will perform allocations until the quotas for all resource types are
satisfied. This can lead to allocations for certain resource type
far exceeding the set quota.
In order to prevent this issue, this patch changes the notion of what
constitutes a "satisfied quota". Where before a quota could only be
satisfied when allocations had been met up to the set quota for every
resource type, we now consider a quota satisfied as soon as at least
one of the resource types is allocated up to the set quota. What this
means is that if the allocation and quota vectors are of the same
proportion, this new technique behaves the same as the previous
technique. However, if the allocation and quota vectors have different
resource type ratios, then the quota becomes satisfied as soon as
the any quota guarantee is hit.
Longer term, we can consider allocating the resource types that have
not reached the quota guarantee, but this is non-trivial (consider
the case of having to allocate only memory, this means we likely
should only offer it in such a manner that it allows an existing
container to increase its memory; it cannot be used to create a
new container unless there is a mechanism to obtain best-effort
cpu/disk).
Also note that the quota guarantee is currently applied as a limit,
whereas longer term we would like to (1) make these distinct, and
(2) distinguish quota and fair sharing via non-revocable and
revocable resources, respectively.
Review: https://reviews.apache.org/r/55625/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/34de30cf
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/34de30cf
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/34de30cf
Branch: refs/heads/master
Commit: 34de30cf69bac40eaa348036d1268bede762d170
Parents: 4dbbf44
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Sun Feb 5 15:45:10 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Sun Feb 5 17:40:27 2017 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 24 +++++++-
src/tests/hierarchical_allocator_tests.cpp | 72 ++++++++++++++++++++++++
2 files changed, 93 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/34de30cf/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index fc93ade..3429a65 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1478,6 +1478,8 @@ void HierarchicalAllocatorProcess::__allocate()
foreach (const string& role, quotaRoleSorter->sort()) {
CHECK(quotas.contains(role));
+ const Quota& quota = quotas.at(role);
+
// If there are no active frameworks in this role, we do not
// need to do any allocations for this role.
if (!activeRoles.contains(role)) {
@@ -1488,14 +1490,30 @@ void HierarchicalAllocatorProcess::__allocate()
// value omits role, reservation, and persistence info.
Resources roleConsumedResources = getQuotaRoleAllocatedResources(role);
- // If quota for the role is satisfied, we do not need to do any further
- // allocations for this role, at least at this stage.
+ // If quota for the role is satisfied, we do not need to do
+ // any further allocations for this role, at least at this
+ // stage. More precisely, we stop allocating if at least
+ // one of the resource guarantees is met. This technique
+ // prevents gaming of the quota allocation, see MESOS-6432.
+ //
+ // Longer term, we could ideally allocate what remains
+ // unsatisfied to allow an existing container to scale
+ // vertically, or to allow the launching of a container
+ // with best-effort cpus/mem/disk/etc.
//
// TODO(alexr): Skipping satisfied roles is pessimistic. Better
// alternatives are:
// * A custom sorter that is aware of quotas and sorts accordingly.
// * Removing satisfied roles from the sorter.
- if (roleConsumedResources.contains(quotas[role].info.guarantee())) {
+ bool someGuaranteesReached = false;
+ foreach (const Resource& guarantee, quota.info.guarantee()) {
+ if (roleConsumedResources.contains(guarantee)) {
+ someGuaranteesReached = true;
+ break;
+ }
+ }
+
+ if (someGuaranteesReached) {
continue;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/34de30cf/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 65fed37..c681d03 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -3649,6 +3649,78 @@ TEST_F(HierarchicalAllocatorTest, ReviveOffers)
}
+// This tests the behavior of quota when the allocation and
+// quota are disproportionate. The current approach (see MESOS-6432)
+// is to stop allocation quota once one of the resource
+// guarantees is reached. We test the following example:
+//
+// Quota: cpus:4;mem:1024
+// Allocation: cpus:2;mem:1024
+//
+// Here no more allocation occurs to the role since the memory
+// guarantee is reached. Longer term, we could offer the
+// unsatisfied cpus to allow an existing container to scale
+// vertically, or to allow the launching of a container with
+// best-effort memory/disk, etc.
+TEST_F(HierarchicalAllocatorTest, DisproportionateQuotaVsAllocation)
+{
+ // Pausing the clock is not necessary, but ensures that the test
+ // doesn't rely on the batch allocation in the allocator, which
+ // would slow down the test.
+ Clock::pause();
+
+ initialize();
+
+ const string QUOTA_ROLE{"quota-role"};
+ const string NO_QUOTA_ROLE{"no-quota-role"};
+
+ // Since resource allocation is coarse-grained, we use a quota such
+ // that mem can be satisfied from a single agent, but cpus requires
+ // multiple agents.
+ const string agentResources = "cpus:2;mem:1024";
+ const string quotaResources = "cpus:4;mem:1024";
+
+ Quota quota = createQuota(QUOTA_ROLE, quotaResources);
+ allocator->setQuota(QUOTA_ROLE, quota);
+
+ // Register two frameworks where on is using a role with quota.
+ FrameworkInfo framework1 = createFrameworkInfo(QUOTA_ROLE);
+ FrameworkInfo framework2 = createFrameworkInfo(NO_QUOTA_ROLE);
+ allocator->addFramework(framework1.id(), framework1, {}, true);
+ allocator->addFramework(framework2.id(), framework2, {}, true);
+
+ // Register an agent. This triggers an allocation of all of the
+ // agent's resources to partially satisfy QUOTA_ROLE's quota. After
+ // the allocation QUOTA_ROLE's quota for mem will be satisfied while
+ // still being below the set quota for cpus. With that QUOTA_ROLE
+ // will not receive more resources since we currently do not
+ // have an ability to offer out only the cpus.
+ SlaveInfo agent1 = createSlaveInfo(agentResources);
+ allocator->addSlave(agent1.id(), agent1, None(), agent1.resources(), {});
+
+ Allocation expected = Allocation(
+ framework1.id(),
+ {{framework1.role(), {{agent1.id(), agent1.resources()}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+
+ // Register a second agent. Since QUOTA_ROLE has reached one
+ // of its quota guarantees and quota currently acts as an upper
+ // limit, no further resources are allocated for quota.
+ // In addition, we "hold back" the resources of the second
+ // agent in order to ensure that the quota could be satisfied
+ // should the first framework decide to consume proportionally
+ // to its quota (e.g. cpus:2;mem:512 on each agent).
+ SlaveInfo agent2 = createSlaveInfo(agentResources);
+ allocator->addSlave(agent2.id(), agent2, None(), agent2.resources(), {});
+
+ Clock::settle();
+
+ Future<Allocation> allocation = allocations.get();
+ EXPECT_TRUE(allocation.isPending());
+}
+
+
class HierarchicalAllocatorTestWithParam
: public HierarchicalAllocatorTestBase,
public WithParamInterface<bool> {};