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/22 23:14:05 UTC
[1/2] mesos git commit: Fixed a quota headroom violation during quota
allocation.
Repository: mesos
Updated Branches:
refs/heads/master c5654bc28 -> 9abb48fb0
Fixed a quota headroom violation during quota allocation.
In the quota role allocation stage, if a role gets some resources
on an agent to meet its quota, it will also get all other resources
on the same agent that it does not have quota for. This may violate
the guarantees of other roles that have set quota on these resources.
We fix this issue during the quota allocation stage by ensuring that,
if a role has no quota set for a scalar resource, it will get that
resource only when two conditions are both met:
(1) It is tentatively being allocated some other resources on the
same agent to meet its quota, or some reservations (which may
or may not involve quota resources); and
(2) After allocating those resources, quota headroom is still
above the required amount.
Also refactored the fine-grained quota allocation logic.
Review: https://reviews.apache.org/r/64793/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0c416d51
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0c416d51
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0c416d51
Branch: refs/heads/master
Commit: 0c416d51940c0ae0f5ae8ca8e9346fa8ab928725
Parents: c5654bc
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Fri Dec 22 14:05:08 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri Dec 22 15:03:11 2017 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 152 ++++++++++++-----------
1 file changed, 80 insertions(+), 72 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/0c416d51/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 78d7b22..20e0b29 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1826,8 +1826,9 @@ void HierarchicalAllocatorProcess::__allocate()
// allocate 10 cpus to the role to keep it within its limit.
//
// In the case that the role needs some of the resources on this
- // agent to make progress towards its quota, we'll *also* allocate
- // all of the resources for which it does not have quota.
+ // agent to make progress towards its quota, or the role is being
+ // allocated some reservation(s), we'll *also* allocate all of
+ // the resources for which it does not have quota.
//
// E.g. The agent has 1 cpu, 1024 mem, 1024 disk, 1 gpu, 5 ports
// and the role has quota for 1 cpu, 1024 mem. We'll include
@@ -1835,13 +1836,9 @@ void HierarchicalAllocatorProcess::__allocate()
// role not having any quota guarantee for them.
//
// We have to do this for now because it's not possible to set
- // quota on non-scalar resources, like ports. However, for scalar
- // resources that can have quota set, we should ideally make
- // sure that when we include them, we're not violating some
- // other role's guarantee!
- //
- // TODO(mzhu): Check the headroom when we're including scalar
- // resources that this role does not have quota for.
+ // quota on non-scalar resources, like ports. For scalar resources
+ // that this role has no quota for, it can be allocated as long
+ // as the quota headroom is not violated.
//
// TODO(mzhu): Since we're treating the resources with unset
// quota as having no guarantee and no limit, these should be
@@ -1858,78 +1855,92 @@ void HierarchicalAllocatorProcess::__allocate()
// Unreserved resources that are tentatively going to be
// allocated towards this role's quota. These resources may
// not get allocated due to framework filters.
- Resources newQuotaAllocation;
-
- if (!unsatisfiedQuota.empty()) {
- Resources unreserved = available.nonRevocable().unreserved();
-
- set<string> quotaResourceNames =
- Resources(quota.info.guarantee()).names();
-
- // When "chopping" resources, there is more than 1 "chop" that
- // can be done to satisfy the limits. Consider the case with
- // two disks of 1GB, one is PATH and another is MOUNT. And a role
- // has a "disk" quota of 1GB. We could pick either of the disks here,
- // but not both.
- //
- // In order to avoid repeatedly choosing the same "chop" of
- // the resources each time we allocate, we introduce some
- // randomness by shuffling the resources.
- google::protobuf::RepeatedPtrField<Resource>
- resourceVector = unreserved;
- random_shuffle(resourceVector.begin(), resourceVector.end());
-
- foreach (const Resource& resource, resourceVector) {
- if (quotaResourceNames.count(resource.name()) == 0) {
- // This resource has no quota set. Allocate it.
- resources += resource;
- continue;
- }
-
- // Currently, we guarantee that quota resources are scalars.
- CHECK_EQ(resource.type(), Value::SCALAR);
+ // These are __quantities__ with no meta-data.
+ Resources newQuotaAllocationScalarQuantities;
- Option<Value::Scalar> newUnsatisfiedQuotaScalar =
- (unsatisfiedQuota -
- newQuotaAllocation.createStrippedScalarQuantity())
- .get<Value::Scalar>(resource.name());
+ // We put resource that this role has no quota for in
+ // `nonQuotaResources` tentatively.
+ Resources nonQuotaResources;
- if (newUnsatisfiedQuotaScalar.isNone()) {
- // Quota for this resource has been satisfied.
- continue;
- }
+ Resources unreserved = available.nonRevocable().unreserved();
- const Value::Scalar& resourceScalar = resource.scalar();
+ set<string> quotaResourceNames =
+ Resources(quota.info.guarantee()).names();
- if (resourceScalar <= newUnsatisfiedQuotaScalar.get()) {
- // We can safely allocate `resource` entirely here without
- // breaking the quota limit.
- resources += resource;
- newQuotaAllocation += resource;
+ // When "chopping" resources, there is more than 1 "chop" that
+ // can be done to satisfy the limits. Consider the case with
+ // two disks of 1GB, one is PATH and another is MOUNT. And a
+ // role has a "disk" quota of 1GB. We could pick either of
+ // the disks here, but not both.
+ //
+ // In order to avoid repeatedly choosing the same "chop" of
+ // the resources each time we allocate, we introduce some
+ // randomness by shuffling the resources.
+ google::protobuf::RepeatedPtrField<Resource>
+ resourceVector = unreserved;
+ random_shuffle(resourceVector.begin(), resourceVector.end());
+
+ foreach (Resource& resource, resourceVector) {
+ if (resource.type() != Value::SCALAR) {
+ // We currently do not support quota for non-scalar resources,
+ // add it to `nonQuotaResources`. See `nonQuotaResources`
+ // regarding how these resources are allocated.
+ nonQuotaResources += resource;
+ continue;
+ }
- continue;
+ if (quotaResourceNames.count(resource.name()) == 0) {
+ // Allocating resource that this role has NO quota for,
+ // the limit concern here is that it should not break the
+ // quota headroom.
+ //
+ // Allocation Limit = Available Headroom - Required Headroom -
+ // Tentative Allocation to Role
+ Resources upperLimitScalarQuantities =
+ availableHeadroom - requiredHeadroom -
+ (newQuotaAllocationScalarQuantities +
+ nonQuotaResources.createStrippedScalarQuantity());
+
+ Option<Value::Scalar> limitScalar =
+ upperLimitScalarQuantities.get<Value::Scalar>(resource.name());
+
+ if (limitScalar.isNone()) {
+ continue; // Already have a headroom deficit.
}
- // At this point, we need to chop `resource` to satisfy
- // the quota. We chop `resource` by making a copy and
- // shrinking its scalar value
- Resource choppedResourceToAllocate = resource;
- choppedResourceToAllocate.mutable_scalar()
- ->CopyFrom(newUnsatisfiedQuotaScalar.get());
-
- // Not all resources are choppable. Some resources such as MOUNT
- // disk has to be offered entirely. We use resources `contains()`
- // utility to verify this. Specifically, if a resource `contains()`
- // a smaller version of itself, then it can be safely chopped.
- if (!Resources(resource).contains(choppedResourceToAllocate)) {
- continue;
+ if (Resources::shrink(&resource, limitScalar.get())) {
+ nonQuotaResources += resource;
+ }
+ } else {
+ // Allocating resource that this role has quota for,
+ // the limit concern is that it should not exceed this
+ // role's unsatisfied quota.
+ Resources upperLimitScalarQuantities =
+ unsatisfiedQuota - newQuotaAllocationScalarQuantities;
+
+ Option<Value::Scalar> limitScalar =
+ upperLimitScalarQuantities.get<Value::Scalar>(resource.name());
+
+ if (limitScalar.isNone()) {
+ continue; // Quota limit already met.
}
- resources += choppedResourceToAllocate;
- newQuotaAllocation += choppedResourceToAllocate;
+ if (Resources::shrink(&resource, limitScalar.get())) {
+ resources += resource;
+ newQuotaAllocationScalarQuantities +=
+ Resources(resource).createStrippedScalarQuantity();
+ }
}
}
+ // We include the non-quota resources (with headroom taken
+ // into account) if this role is being allocated some resources
+ // already: either some quota resources or a reservation
+ // (possibly with quota resources).
+ if (!resources.empty()) {
+ resources += nonQuotaResources;
+ }
+
// It is safe to break here, because all frameworks under a role would
// consider the same resources, so in case we don't have allocatable
// resources, we don't have to check for other frameworks under the
@@ -1976,9 +1987,6 @@ void HierarchicalAllocatorProcess::__allocate()
offerable[frameworkId][role][slaveId] += resources;
offeredSharedResources[slaveId] += resources.shared();
- Resources newQuotaAllocationScalarQuantities =
- newQuotaAllocation.createStrippedScalarQuantity();
-
unsatisfiedQuota -= newQuotaAllocationScalarQuantities;
// Track quota headroom change.
[2/2] mesos git commit: Added a test for quota allocation including
non-quota resources.
Posted by bm...@apache.org.
Added a test for quota allocation including non-quota resources.
This test verifies the behavior of allocating a resource to quota
roles that have no quota set for that particular resource (e.g.
allocating memory to a role with only quota set for CPU). If a
role has no quota set for a resource, it will get that resource
only when two conditions are both met:
(1) It is tentatively being allocated some other resources on the
same agent to meet its quota, or some reservations (which may
or may not involve quota resources); and
(2) After allocating those resources, quota headroom is still
above the required amount.
Review: https://reviews.apache.org/r/64794/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9abb48fb
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9abb48fb
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9abb48fb
Branch: refs/heads/master
Commit: 9abb48fb0cba1be179de148555b5a48dc86408f7
Parents: 0c416d5
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Fri Dec 22 15:05:57 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri Dec 22 15:05:57 2017 -0800
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 105 ++++++++++++++++++++++++
1 file changed, 105 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/9abb48fb/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index ad9d556..9bc939d 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -3492,6 +3492,111 @@ TEST_F(HierarchicalAllocatorTest, QuotaAllocationGranularityUnchoppableResource)
}
+// This test verifies the behavior of allocating a resource to quota roles
+// that has no quota set for that particular resource (e.g. allocating
+// memory to a role with only quota set for CPU). If a role has no quota
+// set for a resource, it will get that resource only when two conditions
+// are both met:
+//
+// - It was allocated quota resources on the agent;
+//
+// - It can be allocated without violating the quota headroom.
+TEST_F(HierarchicalAllocatorTest, QuotaRoleAllocateNonQuotaResource)
+{
+ Clock::pause();
+ initialize();
+
+ const string QUOTA_ROLE_1{"quota-role-1"};
+
+ const Quota quota1 = createQuota(QUOTA_ROLE_1, "cpus:2");
+ allocator->setQuota(QUOTA_ROLE_1, quota1);
+
+ SlaveInfo agent1 = createSlaveInfo("cpus:1;mem:1024;ports:[31000-32000]");
+ allocator->addSlave(
+ agent1.id(),
+ agent1,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent1.resources(),
+ {});
+
+ // Create `framework` under `QUOTA_ROLE_1`.
+ // This will tigger an event-driven allocation loop.
+ FrameworkInfo framework = createFrameworkInfo({QUOTA_ROLE_1});
+ allocator->addFramework(framework.id(), framework, {}, true, {});
+
+ Clock::settle();
+
+ // `framework` will get all resources of `agent1`. Memory resource is
+ // allocated because `QUOTA_ROLE_1` does not have any memory quota limit
+ // and there is no need to set aside memory for other quota roles. Port
+ // resource is allocated due to the same reason (this is further due to
+ // the fact that ports are non-scalar resources that no roles can set
+ // quota for).
+ Allocation expected = Allocation(
+ framework.id(),
+ {{QUOTA_ROLE_1, {{agent1.id(), agent1.resources()}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+
+ // QUOTA_ROLE_1:
+ // quota: "cpus:2"
+ // allocated: "cpus:1;mem:1024;ports:[31000-32000]" (agent1)
+
+ const string QUOTA_ROLE_2{"quota-role-2"};
+
+ const Quota quota2 = createQuota(QUOTA_ROLE_2, "mem:1024");
+ allocator->setQuota(QUOTA_ROLE_2, quota2);
+
+ // Add `agent2` with identical resources.
+ // This will trigger an event-driven allocation on `agent2`.
+ SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:1024;ports:[31000-32000]");
+ allocator->addSlave(
+ agent2.id(),
+ agent2,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent2.resources(),
+ {});
+
+ // `framework` will only get cpu and port resources. CPU resource is
+ // allocated because `QUOTA_ROLE_1` still needs one more CPU to meet its
+ // quota. Memory is NOT allocated even though `QUOTA_ROLE_1` has no memory
+ // quota limit because the memory needs to be set aside for `QUOTA_ROLE_2`.
+ // Port resource is allocated because `QUOTA_ROLE_1` does not have any port
+ // quota limit and there is no need to set aside the port resource for other
+ // quota roles ((this is further due to the fact that ports are non-scalar
+ // resources that no roles can set quota for).
+ expected = Allocation(
+ framework.id(),
+ {{QUOTA_ROLE_1, {{agent2.id(),
+ agent2.resources() - Resources(quota2.info.guarantee())}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+
+ // QUOTA_ROLE_1:
+ // quota: "cpus:2"
+ // allocated: "cpus:1;mem:1024;ports:[31000-32000]" (agent1)
+ // "cpus:1;ports:[31000-32000]" (agent2)
+ //
+ // QUOTA_ROLE_1's quota has been met.
+
+ // Add `agent3` with identical resources.
+ // This will trigger an event-driven allocation on `agent3`.
+ SlaveInfo agent3 = createSlaveInfo("cpus:1;mem:1024;ports:[31000-32000]");
+ allocator->addSlave(
+ agent3.id(),
+ agent3,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent3.resources(),
+ {});
+
+ // No allocation will happen because QUOTA_ROLE_1's quota has been met.
+ EXPECT_TRUE(allocations.get().isPending());
+}
+
+
// This test verifies, that the free pool (what is left after all quotas
// are satisfied) is allocated according to the DRF algorithm across the roles
// which do not have quota set.