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 2018/02/27 04:33:13 UTC
[1/2] mesos git commit: Fixed a bug where quota headroom is
under-calculated.
Repository: mesos
Updated Branches:
refs/heads/master c97ef907f -> a4df5d43e
Fixed a bug where quota headroom is under-calculated.
When calculating the quota headroom, we failed to consider
ancestor's reservation allocated to the child. This leads
to under-calculation of available headroom and excessive
resources being set aside for headroom. See MESOS-8604.
This patches fixes this issue by counting ancestor's reservation
allocated to the child as allocated-reservation even though
the child itself has no reservation.
Review: https://reviews.apache.org/r/65806/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a57c3c9a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a57c3c9a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a57c3c9a
Branch: refs/heads/master
Commit: a57c3c9a7cb1f531d39178086d004975004871e0
Parents: c97ef90
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Mon Feb 26 19:25:09 2018 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Feb 26 19:42:38 2018 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a57c3c9a/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 80e0a34..58aa83f 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1673,30 +1673,34 @@ void HierarchicalAllocatorProcess::__allocate()
roleSorter->allocationScalarQuantities(role).toUnreserved();
}
- // Subtract all unallocated reservations.
- foreachkey (const string& role, reservationScalarQuantities) {
+ // Calculate total allocated reservations. Note that we need to ensure
+ // we count a reservation for "a" being allocated to "a/b", therefore
+ // we cannot simply loop over the reservations' roles.
+ Resources totalAllocatedReservationScalarQuantities;
+ foreachkey (const string& role, roles) {
hashmap<SlaveID, Resources> allocations;
if (quotaRoleSorter->contains(role)) {
allocations = quotaRoleSorter->allocation(role);
} else if (roleSorter->contains(role)) {
allocations = roleSorter->allocation(role);
+ } else {
+ continue; // This role has no allocation.
}
- Resources unallocatedReservations =
- reservationScalarQuantities.get(role).getOrElse(Resources());
-
foreachvalue (const Resources& resources, allocations) {
// NOTE: `totalScalarQuantities` omits dynamic reservation,
// persistent volume info, and allocation info. We additionally
// remove the static reservations here via `toUnreserved()`.
- unallocatedReservations -=
+ totalAllocatedReservationScalarQuantities +=
resources.reserved().createStrippedScalarQuantity().toUnreserved();
}
-
- // Subtract the unallocated reservations for this role from the headroom.
- availableHeadroom -= unallocatedReservations;
}
+ // Subtract total unallocated reservations.
+ availableHeadroom -=
+ Resources::sum(reservationScalarQuantities) -
+ totalAllocatedReservationScalarQuantities;
+
// Subtract revocable resources.
foreachvalue (const Slave& slave, slaves) {
// NOTE: `totalScalarQuantities` omits dynamic reservation,
[2/2] mesos git commit: Added an allocator test for quota with
ancestor reservation.
Posted by bm...@apache.org.
Added an allocator test for quota with ancestor reservation.
This test ensures that resources can be correctly allocated
in the presence of quota when ancestor reservations are
allocated to a descendent role. Specifically, it checks that
ancestor reservations allocated to child are counted by the
quota headroom calculation. See MESOS-8604.
Review: https://reviews.apache.org/r/65807/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a4df5d43
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a4df5d43
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a4df5d43
Branch: refs/heads/master
Commit: a4df5d43e5e029dfe29072f6b59aed57a6176f10
Parents: a57c3c9
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Mon Feb 26 19:42:45 2018 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Feb 26 19:52:10 2018 -0800
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 101 ++++++++++++++++++++++++
1 file changed, 101 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a4df5d43/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 42dc6ac..c97b2ba 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -5515,6 +5515,107 @@ TEST_F(HierarchicalAllocatorTest, OfferAncestorReservationsToDescendantChild)
}
+// This test ensures that resources can be correctly allocated in
+// the presence of quota when a parent role's reservation is allocated
+// to a child role. Specifically, it checks that a parent role's
+// reservations allocated to a child role are correctly considered
+// as an allocated reservation by the quota headroom calculation.
+// See MESOS-8604.
+TEST_F(HierarchicalAllocatorTest, QuotaWithAncestorReservations)
+{
+ Clock::pause();
+
+ initialize();
+
+ const string QUOTA_ROLE{"quota-role"};
+ const string NO_QUOTA_ROLE{"no-quota-role"};
+ const string PARENT_ROLE{"a"};
+ const string CHILD_ROLE{"a/b"};
+
+ Quota quota = createQuota(QUOTA_ROLE, "cpus:1;mem:1024");
+ allocator->setQuota(QUOTA_ROLE, quota);
+
+ // This agent is reserved for the parent role `a`.
+ SlaveInfo agent1 = createSlaveInfo("cpus(a):1;mem(a):1024");
+ allocator->addSlave(
+ agent1.id(),
+ agent1,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent1.resources(),
+ {});
+
+ // Add framework1 under the child role `a/b`.
+ FrameworkInfo framework1 = createFrameworkInfo({CHILD_ROLE});
+ allocator->addFramework(framework1.id(), framework1, {}, true, {});
+
+ // All of agent1's resources are allocated to `framework1` under `a/b`
+ // because it is reserved by its ancestor `a`.
+ Allocation expected = Allocation(
+ framework1.id(),
+ {{CHILD_ROLE, {{agent1.id(), agent1.resources()}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+
+ // Add framework2 under `NO_QUOTA_ROLE`.
+ FrameworkInfo framework2 = createFrameworkInfo({NO_QUOTA_ROLE});
+ allocator->addFramework(framework2.id(), framework2, {}, true, {});
+
+ // No allocations are made because there is no free resources.
+ Future<Allocation> allocation = allocations.get();
+ EXPECT_TRUE(allocation.isPending());
+
+ // Add agent2 and agent3 with the same resources as `quota`.
+
+ Resources agentResources = Resources::parse("cpus:1;mem:1024").get();
+
+ SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:1024");
+ allocator->addSlave(
+ agent2.id(),
+ agent2,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent2.resources(),
+ {});
+
+ SlaveInfo agent3 = createSlaveInfo("cpus:1;mem:1024");
+ allocator->addSlave(
+ agent3.id(),
+ agent3,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent3.resources(),
+ {});
+
+ // Required headroom: quota(cpus:1;mem:1024)
+ //
+ // Available headroom = total resources (cpus:3;mem:3072) -
+ // allocated resources (cpus:1;mem:1024) -
+ // unallocated reservations (cpus:0;mem:0)
+ // = (cpus:2;mem:2048)
+ //
+ // Resources that can allocated without breaking quota headroom =
+ // available headroom - required headroom = (cpus:1;mem:1024)
+
+ // Either agent2 or agent3 will be set aside for the quota headroom for
+ // role `QUOTA_ROLE`. The other will be offered to framework2, because
+ // it hasn't got any resources and has the lowest share.
+ //
+ // Agent2 and agent3 have identical resources. We only care that the
+ // framework can get either of them.
+
+ AWAIT_READY(allocation);
+
+ ASSERT_EQ(allocation->frameworkId, framework2.id());
+ ASSERT_EQ(1u, allocation->resources.size());
+ ASSERT_TRUE(allocation->resources.contains(NO_QUOTA_ROLE));
+
+ agentResources.allocate(NO_QUOTA_ROLE);
+ EXPECT_EQ(agentResources,
+ Resources::sum(allocation->resources.at(NO_QUOTA_ROLE)));
+}
+
+
// This test checks that quota guarantees work as expected when a
// nested role is created as a child of an existing quota'd role.
TEST_F(HierarchicalAllocatorTest, NestedRoleQuota)