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/12/16 02:24:45 UTC
[1/3] mesos git commit: Added a test to check shared resources
accounting in quota enforcement.
Repository: mesos
Updated Branches:
refs/heads/master 91ea75e83 -> f8111469b
Added a test to check shared resources accounting in quota enforcement.
This test verifies that when enforcing quota limit, shared resources
that are part of a role's reserved-allocated resources are only
charged once even when they are offered multiple times.
Review: https://reviews.apache.org/r/64466/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f8111469
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f8111469
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f8111469
Branch: refs/heads/master
Commit: f8111469b2f7c05703a680c1753df3541a83df94
Parents: 2763792
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Fri Dec 15 18:06:07 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri Dec 15 18:22:06 2017 -0800
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 163 ++++++++++++++++++++++++
1 file changed, 163 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f8111469/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 332d4e1..4127d05 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1526,6 +1526,169 @@ TEST_P(HierarchicalAllocatorTestWithReservations,
}
+// This test verifies that when enforcing quota limit, shared resources
+// that are part of a role's reserved-allocated resources are only
+// charged once even when they are offered multiple times.
+TEST_P(HierarchicalAllocatorTestWithReservations,
+ SharedAllocatedResourceQuotaAccounting)
+{
+ // We test this by creating a quota for 100 disk resources.
+ // We then create three agents:
+ //
+ // (1) One agent is reserved for the quota role with 50 disk.
+ // On this agent, we create a shared volume using the 50
+ // disk, and allocate it twice to the framework.
+ //
+ // (2) We create another agent with 50 disk. Since the role
+ // only has 50 disk allocated to it (two instances of the
+ // 50 disk shared volume), the disk should be allocated
+ // towards the role's quota.
+ //
+ // (3) We create another agent with 50 disk. This time, the
+ // agent should not be allocated to the role since now
+ // the role has 100 disk already allocated to it.
+ Clock::pause();
+
+ const string QUOTA_ROLE{"quota-role"};
+
+ initialize();
+
+ const Quota quota = createQuota(QUOTA_ROLE, "cpus:3;mem:2048;disk:100");
+ allocator->setQuota(QUOTA_ROLE, quota);
+
+ Resource::ReservationInfo reservation;
+ reservation.set_type(GetParam());
+ reservation.set_role(QUOTA_ROLE);
+
+ Resources reserved = Resources::parse("cpus:2;mem:1024;disk:50").get();
+ reserved = reserved.pushReservation(reservation);
+
+ SlaveInfo agent1 = createSlaveInfo(reserved);
+ allocator->addSlave(
+ agent1.id(),
+ agent1,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent1.resources(),
+ {});
+
+ FrameworkInfo framework = createFrameworkInfo(
+ {QUOTA_ROLE},
+ {FrameworkInfo::Capability::SHARED_RESOURCES});
+
+ allocator->addFramework(framework.id(), framework, {}, true, {});
+
+ Allocation expected = Allocation(
+ framework.id(),
+ {{QUOTA_ROLE, {{agent1.id(), reserved}}}});
+
+ Future<Allocation> allocation = allocations.get();
+ AWAIT_EXPECT_EQ(expected, allocation);
+
+ // Quota: "cpus:3;mem:2048;disk:100".
+ // Allocated quota: "cpus:2;mem:1024;disk:50".
+
+ Resource::AllocationInfo allocationInfo;
+ allocationInfo.set_role(QUOTA_ROLE);
+
+ // Create a shared volume.
+ Resource volume = createDiskResource(
+ "50", QUOTA_ROLE, "id1", None(), None(), true);
+
+ // Inject reservation info.
+ volume.add_reservations()->CopyFrom(reservation);
+
+ Offer::Operation create = CREATE(volume);
+ protobuf::injectAllocationInfo(&create, allocationInfo);
+
+ Try<vector<ResourceConversion>> conversions = getResourceConversions(create);
+ ASSERT_SOME(conversions);
+
+ // Ensure the CREATE operation can be applied.
+ Try<Resources> updated =
+ allocation->resources.at(QUOTA_ROLE).at(agent1.id())
+ .apply(conversions.get());
+
+ ASSERT_SOME(updated);
+
+ // Update the allocation in the allocator with a CREATE operation.
+ allocator->updateAllocation(
+ framework.id(),
+ agent1.id(),
+ allocation->resources.at(QUOTA_ROLE).at(agent1.id()),
+ conversions.get());
+
+ // Recover part of the resources for the next allocation.
+ Resources recover = allocatedResources(
+ Resources::parse("cpus:1;mem:512").get(),
+ QUOTA_ROLE);
+
+ // Inject reservation info.
+ recover = recover.pushReservation(reservation);
+
+ allocator->recoverResources(
+ framework.id(),
+ agent1.id(),
+ recover,
+ None());
+
+ // Quota: "cpus:3;mem:2048;disk:100".
+ // Allocated quota: "cpus:1;mem:512;disk:50".
+
+ // Trigger a batch allocation.
+ Clock::advance(flags.allocation_interval);
+
+ // The remaining resources (cpus:1;mem:512) of agent1 along with
+ // the shared volume are offered again.
+ expected = Allocation(
+ framework.id(),
+ {{QUOTA_ROLE, {{agent1.id(), recover + create.create().volumes()}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+
+ // Quota: "cpus:3;mem:2048;disk:100".
+ // Allocated quota: "cpus:2;mem:1024;disk:50".
+ // Note: 50 shared disk is allocated twice but should only count once.
+
+ // Add agent2. This will trigger a batch allocation.
+ SlaveInfo agent2 = createSlaveInfo("cpus:0.5;mem:512;disk:50");
+ allocator->addSlave(
+ agent2.id(),
+ agent2,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent2.resources(),
+ {});
+
+ // `framework` will get all of agent2's resources.
+ expected = Allocation(
+ framework.id(),
+ {{QUOTA_ROLE, {{agent2.id(), agent2.resources()}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+
+ // Quota: "cpus:3;mem:2048;disk:100"
+ // Allocated quota: "cpus:2.5;mem:1536;disk:100"
+ // The quota is met (disk limit has been reached).
+
+ // Add agent3.
+ // This will trigger a batch allocation.
+ SlaveInfo agent3 = createSlaveInfo("cpus:0.5;mem:512;disk:50");
+ allocator->addSlave(
+ agent3.id(),
+ agent3,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent3.resources(),
+ {});
+
+ Clock::settle();
+
+ // No more allocations because quota limit has been reached.
+ EXPECT_TRUE(allocations.get().isPending());
+}
+
+
// Checks that resources on a slave that are statically reserved to
// a role are only offered to frameworks in that role.
TEST_F(HierarchicalAllocatorTest, Reservations)
[2/3] mesos git commit: Added a test to ensure non-quota role
reservations are allocated.
Posted by bm...@apache.org.
Added a test to ensure non-quota role reservations are allocated.
This test checks against the symptom of failing to allocate
non-quota role's reservations if resources are set aside for
the quota limit headroom. See MESOS-8293.
Review: https://reviews.apache.org/r/64465/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2763792a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2763792a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2763792a
Branch: refs/heads/master
Commit: 2763792a16cb2a7579c0a150ed4d3ac523d92883
Parents: 4fa9092
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Fri Dec 15 17:51:54 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri Dec 15 18:22:06 2017 -0800
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 78 +++++++++++++++++++++++++
1 file changed, 78 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2763792a/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index f5fb47e..332d4e1 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1448,6 +1448,84 @@ TEST_P(HierarchicalAllocatorTestWithReservations, ReservationAllocated)
}
+// This test verifies that the non-quota role can get its reservation allocated
+// when it co-exists with roles with unsatisfied quota. See MESOS-8293.
+TEST_P(HierarchicalAllocatorTestWithReservations,
+ NonQuotaRoleReservationWithQuotaRole)
+{
+ Clock::pause();
+
+ const string QUOTA_ROLE{"quota-role"};
+ const string NON_QUOTA_ROLE{"non-quota-role"};
+
+ initialize();
+
+ SlaveInfo agent1;
+
+ Resources unreserved = Resources::parse("cpus:2;mem:2048").get();
+
+ Resource::ReservationInfo reservation;
+ reservation.set_type(GetParam());
+ reservation.set_role(NON_QUOTA_ROLE);
+
+ Resources reserved = unreserved.pushReservation(reservation);
+
+ agent1 = createSlaveInfo(unreserved + reserved);
+ allocator->addSlave(
+ agent1.id(),
+ agent1,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent1.resources(),
+ {});
+
+ const Quota quota = createQuota(QUOTA_ROLE, "cpus:2;mem:2048");
+ allocator->setQuota(QUOTA_ROLE, quota);
+
+ // Create `framework1` and set quota to half the size of agent1' resources
+ // for its role.
+ FrameworkInfo framework1 = createFrameworkInfo({QUOTA_ROLE});
+ allocator->addFramework(framework1.id(), framework1, {}, true, {});
+
+ // Process all triggered allocation events.
+ Clock::settle();
+
+ // `framework1` will be offered unreserved resources of `agent1`
+ // because it is the only framework.
+ Allocation expected = Allocation(
+ framework1.id(),
+ {{QUOTA_ROLE, {{agent1.id(), unreserved}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+
+ // Decline resources on agent1 for the rest of the test.
+ Filters filter1day;
+ filter1day.set_refuse_seconds(Days(1).secs());
+ allocator->recoverResources(
+ framework1.id(),
+ agent1.id(),
+ allocatedResources(unreserved, QUOTA_ROLE),
+ filter1day);
+
+ // Create `framework2` which belongs to the `NON_QUOTA_ROLE`
+ // and is entitled to its reserved resources.
+ // This will trigger a batch allocation.
+ FrameworkInfo framework2 = createFrameworkInfo({NON_QUOTA_ROLE});
+ allocator->addFramework(framework2.id(), framework2, {}, true, {});
+
+ Clock::settle();
+
+ // `framework2` will be offered its reserved resources on `agent1`
+ // even though half of agent1's resources are set aside for the quota
+ // headroom.
+ expected = Allocation(
+ framework2.id(),
+ {{NON_QUOTA_ROLE, {{agent1.id(), reserved}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+}
+
+
// Checks that resources on a slave that are statically reserved to
// a role are only offered to frameworks in that role.
TEST_F(HierarchicalAllocatorTest, Reservations)
[3/3] mesos git commit: Fixed a bug with quota headroom that can
leave reservations unallocated.
Posted by bm...@apache.org.
Fixed a bug with quota headroom that can leave reservations unallocated.
Also fixed a bug where quota headroom may include revocable resources.
Review: https://reviews.apache.org/r/64636/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4fa90924
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4fa90924
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4fa90924
Branch: refs/heads/master
Commit: 4fa909245c1ec8a4bc50f9867164be1f1e3653ef
Parents: 91ea75e
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Fri Dec 15 14:52:22 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri Dec 15 18:22:06 2017 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 168 +++++++++++++----------
1 file changed, 97 insertions(+), 71 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fa90924/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index fccd71c..2cabafe 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1819,73 +1819,96 @@ void HierarchicalAllocatorProcess::__allocate()
}
}
- // Calculate the total quantity of scalar resources (including revocable
- // and reserved) that are available for allocation in the next round. We
- // need this in order to ensure we do not over-allocate resources during
- // the second stage.
- //
- // For performance reasons (MESOS-4833), this omits information about
- // dynamic reservations or persistent volumes in the resources.
- //
- // NOTE: We use total cluster resources, and not just those based on the
- // agents participating in the current allocation (i.e. provided as an
- // argument to the `allocate()` call) so that frameworks in roles without
- // quota are not unnecessarily deprived of resources.
- Resources remainingClusterResources = roleSorter->totalScalarQuantities();
- foreachkey (const string& role, roles) {
- remainingClusterResources -= roleSorter->allocationScalarQuantities(role);
- }
-
// Frameworks in a quota'ed role may temporarily reject resources by
- // filtering or suppressing offers. Hence quotas may not be fully allocated.
- Resources unallocatedQuotaResources;
- foreachpair (const string& name, const Quota& quota, quotas) {
+ // filtering or suppressing offers. Hence, quotas may not be fully
+ // allocated and if so we need to hold back some headroom to ensure
+ // the quota can later be satisfied when needed.
+ Resources requiredHeadroom;
+ foreachpair (const string& role, const Quota& quota, quotas) {
// Compute the amount of quota that the role does not have allocated.
//
// NOTE: Revocable resources are excluded in `quotaRoleSorter`.
// NOTE: Only scalars are considered for quota.
- Resources allocated = getQuotaRoleAllocatedResources(name);
+ Resources allocated = getQuotaRoleAllocatedResources(role);
const Resources required = quota.info.guarantee();
- unallocatedQuotaResources += (required - allocated);
+ requiredHeadroom += (required - allocated);
}
- // Determine how many resources we may allocate during the next stage.
+ // The amount of headroom currently available consists of
+ // unallocated resources that can be allocated to satisfy quota.
+ // What that means is that the resources need to be:
//
- // NOTE: Resources for quota allocations are already accounted in
- // `remainingClusterResources`.
- remainingClusterResources -= unallocatedQuotaResources;
-
- // Shared resources are excluded in determination of over-allocation of
- // available resources since shared resources are always allocatable.
- remainingClusterResources = remainingClusterResources.nonShared();
-
- // To ensure we do not over-allocate resources during the second stage
- // with all frameworks, we use 2 stopping criteria:
- // * No available resources for the second stage left, i.e.
- // `remainingClusterResources` - `allocatedStage2` is empty.
- // * A potential offer will force the second stage to use more resources
- // than available, i.e. `remainingClusterResources` does not contain
- // (`allocatedStage2` + potential offer). In this case we skip this
- // agent and continue to the next one.
+ // (1) Non-revocable
+ // (2) Unreserved, or reserved to a quota role.
//
- // NOTE: Like `remainingClusterResources`, `allocatedStage2` omits
- // information about dynamic reservations and persistent volumes for
- // performance reasons. This invariant is preserved because we only add
- // resources to it that have also had this metadata stripped from them
- // (typically by using `Resources::createStrippedScalarQuantity`).
- Resources allocatedStage2;
-
- // At this point resources for quotas are allocated or accounted for.
- // Proceed with allocating the remaining free pool.
- foreach (const SlaveID& slaveId, slaveIds) {
- // If there are no resources available for the second stage, stop.
- //
- // TODO(mzhu): Breaking early here means that we may leave reservations
- // unallocated. See MESOS-8293.
- if (!allocatable(remainingClusterResources - allocatedStage2)) {
- break;
+ // TODO(bmahler): Note that we currently do not consider quota
+ // headroom on an per-role basis, which means we may not hold
+ // back sufficient headroom for a particular quota role if some
+ // other quota roles have more reservations than quota.
+ // See MESOS-8339.
+ //
+ // We will allocate resources while ensuring that the required
+ // headroom is still available. Otherwise we will not be able
+ // to satisfy quota later. Reservations to non-quota roles and
+ // revocable resources will always be included in the offers
+ // since these are not part of the available headroom for quota.
+ //
+ // available headroom = unallocated resources -
+ // unallocated non-quota role reservations -
+ // unallocated revocable resources
+
+ // NOTE: `totalScalarQuantities` omits dynamic reservation,
+ // persistent volume info, and allocation info. We additionally
+ // remove the static reservations here via `toUnreserved()`.
+ Resources availableHeadroom =
+ roleSorter->totalScalarQuantities().toUnreserved();
+
+ // Subtract allocated resources from the total.
+ foreachkey (const string& role, roles) {
+ // NOTE: `totalScalarQuantities` omits dynamic reservation,
+ // persistent volume info, and allocation info. We additionally
+ // remove the static reservations here via `toUnreserved()`.
+ availableHeadroom -=
+ roleSorter->allocationScalarQuantities(role).toUnreserved();
+ }
+
+ // Subtract non quota role reservations.
+ foreachkey (const string& role, reservationScalarQuantities) {
+ if (quotas.contains(role)) {
+ continue; // Skip quota roles.
+ }
+
+ hashmap<SlaveID, Resources> allocations;
+ if (roleSorter->contains(role)) {
+ allocations = roleSorter->allocation(role);
+ }
+
+ Resources unallocatedNonQuotaRoleReservations =
+ 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()`.
+ unallocatedNonQuotaRoleReservations -=
+ resources.reserved().createStrippedScalarQuantity().toUnreserved();
}
+ // Subtract the unallocated reservations for this non quota
+ // role from the headroom.
+ availableHeadroom -= unallocatedNonQuotaRoleReservations;
+ }
+
+ // Subtract revocable resources.
+ foreachvalue (const Slave& slave, slaves) {
+ // NOTE: `totalScalarQuantities` omits dynamic reservation,
+ // persistent volume info, and allocation info. We additionally
+ // remove the static reservations here via `toUnreserved()`.
+ availableHeadroom -= slave.available().revocable()
+ .createStrippedScalarQuantity().toUnreserved();
+ }
+
+ foreach (const SlaveID& slaveId, slaveIds) {
foreach (const string& role, roleSorter->sort()) {
// In the second allocation stage, we only allocate
// for non-quota roles.
@@ -1982,6 +2005,20 @@ void HierarchicalAllocatorProcess::__allocate()
});
}
+ // If allocating these resources would reduce the headroom
+ // below what is required, we will hold them back.
+ const Resources headroomToAllocate = resources
+ .scalars().unreserved().nonRevocable();
+
+ bool sufficientHeadroom =
+ (availableHeadroom -
+ headroomToAllocate.createStrippedScalarQuantity())
+ .contains(requiredHeadroom);
+
+ if (!sufficientHeadroom) {
+ resources -= headroomToAllocate;
+ }
+
// If the resources are not allocatable, ignore. We cannot break
// here, because another framework under the same role could accept
// revocable resources and breaking would skip all other frameworks.
@@ -1994,21 +2031,6 @@ void HierarchicalAllocatorProcess::__allocate()
continue;
}
- // If the offer generated by `resources` would force the second
- // stage to use more than `remainingClusterResources`, move along.
- // We do not terminate early, as offers generated further in the
- // loop may be small enough to fit within `remainingClusterResources`.
- //
- // We exclude shared resources from over-allocation check because
- // shared resources are always allocatable.
- const Resources scalarQuantity =
- resources.nonShared().createStrippedScalarQuantity();
-
- if (!remainingClusterResources.contains(
- allocatedStage2 + scalarQuantity)) {
- continue;
- }
-
VLOG(2) << "Allocating " << resources << " on agent " << slaveId
<< " to role " << role << " of framework " << frameworkId;
@@ -2021,7 +2043,11 @@ void HierarchicalAllocatorProcess::__allocate()
// agent as part of quota.
offerable[frameworkId][role][slaveId] += resources;
offeredSharedResources[slaveId] += resources.shared();
- allocatedStage2 += scalarQuantity;
+
+ if (sufficientHeadroom) {
+ availableHeadroom -=
+ headroomToAllocate.createStrippedScalarQuantity();
+ }
slave.allocated += resources;