You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mp...@apache.org on 2016/01/27 05:15:53 UTC
[1/7] mesos git commit: Updated the allocator to not offer unreserved
resources beyond quota.
Repository: mesos
Updated Branches:
refs/heads/master ab9927339 -> 2f9e91e06
Updated the allocator to not offer unreserved resources beyond quota.
Eventually, we will also want to offer unreserved resources as revocable beyond
the role's quota. In the short term, we do not offer unreserved, non-revocable
resources beyond a role's quota.
Review: https://reviews.apache.org/r/42835
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7f161870
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7f161870
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7f161870
Branch: refs/heads/master
Commit: 7f161870d21b38dfea0ecbd75eb52a7c407b22cb
Parents: 77f003a
Author: Michael Park <mp...@apache.org>
Authored: Tue Jan 26 17:07:23 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Tue Jan 26 20:06:28 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 12 +++++++++++-
src/tests/hierarchical_allocator_tests.cpp | 11 ++++++-----
2 files changed, 17 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/7f161870/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 63de667..29856e1 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1337,7 +1337,17 @@ void HierarchicalAllocatorProcess::allocate(
//
// NOTE: Currently, frameworks are allowed to have '*' role.
// Calling reserved('*') returns an empty Resources object.
- Resources resources = available.unreserved() + available.reserved(role);
+ //
+ // NOTE: We do not offer roles with quota any more non-revocable
+ // resources once their quota is satisfied. However, note that this is
+ // not strictly true due to the coarse-grained nature (per agent) of the
+ // allocation algorithm in stage 1.
+ //
+ // TODO(mpark): Offer unreserved resources as revocable beyond quota.
+ Resources resources = available.reserved(role);
+ if (!quotas.contains(role)) {
+ resources += available.unreserved();
+ }
// Remove revocable resources if the framework has not opted
// for them.
http://git-wip-us.apache.org/repos/asf/mesos/blob/7f161870/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index e4c26ab..f18e6eb 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1806,7 +1806,8 @@ TEST_F(HierarchicalAllocatorTest, QuotaAllocationGranularity)
// This test verifies, that the free pool (what is left after all quotas
-// are satisfied) is allocated according to the DRF algorithm.
+// are satisfied) is allocated according to the DRF algorithm across the roles
+// which do not have quota set.
TEST_F(HierarchicalAllocatorTest, DRFWithQuota)
{
// Pausing the clock is not necessary, but ensures that the test
@@ -1885,12 +1886,12 @@ TEST_F(HierarchicalAllocatorTest, DRFWithQuota)
agent2.resources(),
hashmap<FrameworkID, Resources>());
- // `framework1` will be offered all of `agent2`'s resources
- // (coarse-grained allocation) because its share is less than
- // `framework2`'s share.
+ // `framework2` will be offered all of `agent2`'s resources (coarse-grained
+ // allocation). `framework1` does not receive them even though it has a
+ // smaller allocation, since we have already satisfied its role's quota.
allocation = allocations.get();
AWAIT_READY(allocation);
- EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
+ EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
EXPECT_EQ(agent2.resources(), Resources::sum(allocation.get().resources));
}
[4/7] mesos git commit: Included reserved resources in the role
sorter for DRF.
Posted by mp...@apache.org.
Included reserved resources in the role sorter for DRF.
Review: https://reviews.apache.org/r/42711
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1f17e8c5
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1f17e8c5
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1f17e8c5
Branch: refs/heads/master
Commit: 1f17e8c5b292982f117da25ea931924ee90a3b24
Parents: ab99273
Author: Michael Park <mp...@apache.org>
Authored: Sun Jan 24 22:24:33 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Tue Jan 26 20:06:28 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 25 +++++++++++-------------
src/master/allocator/mesos/hierarchical.hpp | 17 +++++++++++-----
src/tests/hierarchical_allocator_tests.cpp | 12 ++++--------
3 files changed, 27 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f17e8c5/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index fa99397..bf45e05 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -238,7 +238,7 @@ void HierarchicalAllocatorProcess::addFramework(
// Update the allocation for this framework.
foreachpair (const SlaveID& slaveId, const Resources& allocated, used) {
- roleSorter->allocated(role, slaveId, allocated.unreserved());
+ roleSorter->allocated(role, slaveId, allocated);
frameworkSorters[role]->add(slaveId, allocated);
frameworkSorters[role]->allocated(frameworkId.value(), slaveId, allocated);
@@ -286,7 +286,7 @@ void HierarchicalAllocatorProcess::removeFramework(
// Update the allocation for this framework.
foreachpair (
const SlaveID& slaveId, const Resources& allocated, allocation) {
- roleSorter->unallocated(role, slaveId, allocated.unreserved());
+ roleSorter->unallocated(role, slaveId, allocated);
frameworkSorters[role]->remove(slaveId, allocated);
if (quotas.contains(role)) {
@@ -410,7 +410,7 @@ void HierarchicalAllocatorProcess::addSlave(
CHECK(!slaves.contains(slaveId));
CHECK(!paused || expectedAgentCount.isSome());
- roleSorter->add(slaveId, total.unreserved());
+ roleSorter->add(slaveId, total);
quotaRoleSorter->add(slaveId, total.unreserved().nonRevocable());
// Update the allocation for each framework.
@@ -425,7 +425,7 @@ void HierarchicalAllocatorProcess::addSlave(
CHECK(roleSorter->contains(role));
CHECK(frameworkSorters.contains(role));
- roleSorter->allocated(role, slaveId, allocated.unreserved());
+ roleSorter->allocated(role, slaveId, allocated);
frameworkSorters[role]->add(slaveId, allocated);
frameworkSorters[role]->allocated(
frameworkId.value(), slaveId, allocated);
@@ -488,7 +488,7 @@ void HierarchicalAllocatorProcess::removeSlave(
// all the resources. Fixing this would require more information
// than what we currently track in the allocator.
- roleSorter->remove(slaveId, slaves[slaveId].total.unreserved());
+ roleSorter->remove(slaveId, slaves[slaveId].total);
quotaRoleSorter->remove(
slaveId, slaves[slaveId].total.unreserved().nonRevocable());
@@ -520,7 +520,7 @@ void HierarchicalAllocatorProcess::updateSlave(
slaves[slaveId].total = slaves[slaveId].total.nonRevocable() + oversubscribed;
// Now, update the total resources in the role sorters.
- roleSorter->update(slaveId, slaves[slaveId].total.unreserved());
+ roleSorter->update(slaveId, slaves[slaveId].total);
quotaRoleSorter->update(
slaveId, slaves[slaveId].total.unreserved().nonRevocable());
@@ -621,8 +621,8 @@ void HierarchicalAllocatorProcess::updateAllocation(
roleSorter->update(
role,
slaveId,
- frameworkAllocation.unreserved(),
- updatedFrameworkAllocation.get().unreserved());
+ frameworkAllocation,
+ updatedFrameworkAllocation.get());
if (quotas.contains(role)) {
quotaRoleSorter->update(
@@ -685,7 +685,7 @@ Future<Nothing> HierarchicalAllocatorProcess::updateAvailable(
slaves[slaveId].total = updatedTotal.get();
// Now, update the total resources in the role sorters.
- roleSorter->update(slaveId, slaves[slaveId].total.unreserved());
+ roleSorter->update(slaveId, slaves[slaveId].total);
quotaRoleSorter->update(
slaveId, slaves[slaveId].total.unreserved().nonRevocable());
@@ -864,7 +864,7 @@ void HierarchicalAllocatorProcess::recoverResources(
frameworkSorters[role]->unallocated(
frameworkId.value(), slaveId, resources);
frameworkSorters[role]->remove(slaveId, resources);
- roleSorter->unallocated(role, slaveId, resources.unreserved());
+ roleSorter->unallocated(role, slaveId, resources);
if (quotas.contains(role)) {
quotaRoleSorter->unallocated(
@@ -1341,12 +1341,9 @@ void HierarchicalAllocatorProcess::allocate(
allocatedStage2 += resources;
slaves[slaveId].allocated += resources;
- // Reserved resources are only accounted for in the framework
- // sorter, since the reserved resources are not shared across
- // roles.
frameworkSorters[role]->add(slaveId, resources);
frameworkSorters[role]->allocated(frameworkId_, slaveId, resources);
- roleSorter->allocated(role, slaveId, resources.unreserved());
+ roleSorter->allocated(role, slaveId, resources);
if (quotas.contains(role)) {
quotaRoleSorter->allocated(
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f17e8c5/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 2d01034..39b2775 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -392,12 +392,19 @@ protected:
//
// Each stage comprises two levels of sorting, hence "hierarchical".
// Level 1 sorts across roles:
- // Reserved resources are excluded from fairness calculation,
- // since they are forcibly pinned to a role.
+ // Currently, only the allocated portion of the reserved resources are
+ // accounted for fairness calculation.
+ //
+ // TODO(mpark): Reserved resources should be accounted for fairness
+ // calculation whether they are allocated or not, since they model a long or
+ // forever running task. That is, the effect of reserving resources is
+ // equivalent to launching a task in that the resources that make up the
+ // reservation are not available to other roles as non-revocable.
+ //
// Level 2 sorts across frameworks within a particular role:
- // Both reserved resources and unreserved resources are used
- // in the fairness calculation. This is because reserved
- // resources can be allocated to any framework in the role.
+ // Reserved resources at this level are, and should be accounted for
+ // fairness calculation only if they are allocated. This is because
+ // reserved resources are fairly shared across the frameworks in the role.
//
// The allocator relies on `Sorter`s to employ a particular sorting
// algorithm. Each level has its own sorter and hence may have different
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f17e8c5/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 7d5eed4..5777dd7 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -373,9 +373,7 @@ TEST_F(HierarchicalAllocatorTest, UnreservedDRF)
}
-// This test ensures that reserved resources do not affect the sharing
-// across roles. However, reserved resources should be shared fairly
-// *within* a role.
+// This test ensures that reserved resources do affect the sharing across roles.
TEST_F(HierarchicalAllocatorTest, ReservedDRF)
{
// Pausing the clock is not necessary, but ensures that the test
@@ -415,16 +413,14 @@ TEST_F(HierarchicalAllocatorTest, ReservedDRF)
EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
EXPECT_EQ(slave2.resources(), Resources::sum(allocation.get().resources));
- // Now, even though framework1 has more resources allocated to
- // it than framework2, reserved resources are not considered for
- // fairness across roles! We expect framework1 to receive this
- // slave's resources, since it has fewer unreserved resources.
+ // Since `framework1` has more resources allocated to it than `framework2`,
+ // We expect `framework2` to receive this agent's resources.
SlaveInfo slave3 = createSlaveInfo("cpus:2;mem:512;disk:0");
allocator->addSlave(slave3.id(), slave3, None(), slave3.resources(), EMPTY);
allocation = allocations.get();
AWAIT_READY(allocation);
- EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
+ EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
EXPECT_EQ(slave3.resources(), Resources::sum(allocation.get().resources));
// Now add another framework in role1. Since the reserved resources
[3/7] mesos git commit: Included reserved resources in the quota role
sorter for DRF.
Posted by mp...@apache.org.
Included reserved resources in the quota role sorter for DRF.
Review: https://reviews.apache.org/r/42738
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2339984d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2339984d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2339984d
Branch: refs/heads/master
Commit: 2339984dc0f6873e686d08233737481ed747df00
Parents: 1f17e8c
Author: Michael Park <mp...@apache.org>
Authored: Fri Jan 22 22:52:38 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Tue Jan 26 20:06:28 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 56 +++++++++++++-----------
src/master/allocator/mesos/hierarchical.hpp | 8 ++--
2 files changed, 36 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2339984d/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index bf45e05..02fe9fe 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -243,8 +243,8 @@ void HierarchicalAllocatorProcess::addFramework(
frameworkSorters[role]->allocated(frameworkId.value(), slaveId, allocated);
if (quotas.contains(role)) {
- quotaRoleSorter->allocated(
- role, slaveId, allocated.unreserved().nonRevocable());
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->allocated(role, slaveId, allocated.nonRevocable());
}
}
@@ -290,8 +290,8 @@ void HierarchicalAllocatorProcess::removeFramework(
frameworkSorters[role]->remove(slaveId, allocated);
if (quotas.contains(role)) {
- quotaRoleSorter->unallocated(
- role, slaveId, allocated.unreserved().nonRevocable());
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->unallocated(role, slaveId, allocated.nonRevocable());
}
}
@@ -411,7 +411,9 @@ void HierarchicalAllocatorProcess::addSlave(
CHECK(!paused || expectedAgentCount.isSome());
roleSorter->add(slaveId, total);
- quotaRoleSorter->add(slaveId, total.unreserved().nonRevocable());
+
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->add(slaveId, total.nonRevocable());
// Update the allocation for each framework.
foreachpair (const FrameworkID& frameworkId,
@@ -431,8 +433,8 @@ void HierarchicalAllocatorProcess::addSlave(
frameworkId.value(), slaveId, allocated);
if (quotas.contains(role)) {
- quotaRoleSorter->allocated(
- role, slaveId, allocated.unreserved().nonRevocable());
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->allocated(role, slaveId, allocated.nonRevocable());
}
}
}
@@ -489,8 +491,9 @@ void HierarchicalAllocatorProcess::removeSlave(
// than what we currently track in the allocator.
roleSorter->remove(slaveId, slaves[slaveId].total);
- quotaRoleSorter->remove(
- slaveId, slaves[slaveId].total.unreserved().nonRevocable());
+
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->remove(slaveId, slaves[slaveId].total.nonRevocable());
slaves.erase(slaveId);
@@ -521,8 +524,9 @@ void HierarchicalAllocatorProcess::updateSlave(
// Now, update the total resources in the role sorters.
roleSorter->update(slaveId, slaves[slaveId].total);
- quotaRoleSorter->update(
- slaveId, slaves[slaveId].total.unreserved().nonRevocable());
+
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->update(slaveId, slaves[slaveId].total.nonRevocable());
LOG(INFO) << "Slave " << slaveId << " (" << slaves[slaveId].hostname << ")"
<< " updated with oversubscribed resources " << oversubscribed
@@ -625,11 +629,12 @@ void HierarchicalAllocatorProcess::updateAllocation(
updatedFrameworkAllocation.get());
if (quotas.contains(role)) {
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
quotaRoleSorter->update(
role,
slaveId,
- frameworkAllocation.unreserved().nonRevocable(),
- updatedFrameworkAllocation.get().unreserved().nonRevocable());
+ frameworkAllocation.nonRevocable(),
+ updatedFrameworkAllocation.get().nonRevocable());
}
Try<Resources> updatedSlaveAllocation =
@@ -686,8 +691,9 @@ Future<Nothing> HierarchicalAllocatorProcess::updateAvailable(
// Now, update the total resources in the role sorters.
roleSorter->update(slaveId, slaves[slaveId].total);
- quotaRoleSorter->update(
- slaveId, slaves[slaveId].total.unreserved().nonRevocable());
+
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->update(slaveId, slaves[slaveId].total.nonRevocable());
return Nothing();
}
@@ -867,8 +873,8 @@ void HierarchicalAllocatorProcess::recoverResources(
roleSorter->unallocated(role, slaveId, resources);
if (quotas.contains(role)) {
- quotaRoleSorter->unallocated(
- role, slaveId, resources.unreserved().nonRevocable());
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->unallocated(role, slaveId, resources.nonRevocable());
}
}
}
@@ -1009,8 +1015,8 @@ void HierarchicalAllocatorProcess::setQuota(
hashmap<SlaveID, Resources> roleAllocation = roleSorter->allocation(role);
foreachpair (
const SlaveID& slaveId, const Resources& resources, roleAllocation) {
- quotaRoleSorter->allocated(
- role, slaveId, resources.unreserved().nonRevocable());
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->allocated(role, slaveId, resources.nonRevocable());
}
}
@@ -1159,8 +1165,7 @@ void HierarchicalAllocatorProcess::allocate(
// Summing up resources is fine because quota is only for scalar
// resources.
//
- // NOTE: Reserved and revocable resources are excluded in
- // `quotaRoleSorter`.
+ // NOTE: Revocable resources are excluded in `quotaRoleSorter`.
//
// TODO(alexr): Consider including dynamically reserved resources.
Resources roleConsumedResources =
@@ -1221,7 +1226,7 @@ void HierarchicalAllocatorProcess::allocate(
// Resources allocated as part of the quota count towards the
// role's and the framework's fair share.
//
- // NOTE: Reserved and revocable resources have already been excluded.
+ // NOTE: Revocable resources have already been excluded.
frameworkSorters[role]->add(slaveId, resources);
frameworkSorters[role]->allocated(frameworkId_, slaveId, resources);
roleSorter->allocated(role, slaveId, resources);
@@ -1252,7 +1257,7 @@ void HierarchicalAllocatorProcess::allocate(
foreachpair (const string& name, const Quota& quota, quotas) {
// Compute the amount of quota that the role does not have allocated.
//
- // NOTE: Reserved and revocable resources are excluded in `quotaRoleSorter`.
+ // NOTE: Revocable resources are excluded in `quotaRoleSorter`.
Resources allocated = Resources::sum(quotaRoleSorter->allocation(name));
const Resources required = quota.info.guarantee();
unallocatedQuotaResources += (required - allocated);
@@ -1346,8 +1351,9 @@ void HierarchicalAllocatorProcess::allocate(
roleSorter->allocated(role, slaveId, resources);
if (quotas.contains(role)) {
- quotaRoleSorter->allocated(
- role, slaveId, resources.unreserved().nonRevocable());
+ // See comment at `quotaRoleSorter` declaration regarding
+ // non-revocable.
+ quotaRoleSorter->allocated(role, slaveId, resources.nonRevocable());
}
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/2339984d/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 39b2775..1f0c440 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -431,9 +431,11 @@ protected:
// `roleSorter` only contains entries for roles with one or more
// registered frameworks.
//
- // NOTE: This sorter counts only unreserved non-revocable resources.
- //
- // TODO(alexr): Consider including dynamically reserved resources.
+ // NOTE: We do not include revocable resources in the quota role sorter,
+ // because the quota role sorter's job is to perform fair sharing between
+ // the quota roles as it pertains to their level of quota satisfaction.
+ // Since revocable resources do not increase a role's level of satisfaction
+ // toward its quota, we choose to exclude them from the quota role sorter.
Sorter* quotaRoleSorter;
// A collection of sorters, one per active role. Each sorter determines
[6/7] mesos git commit: Accounted for reserved resources in the quota
guarantee check.
Posted by mp...@apache.org.
Accounted for reserved resources in the quota guarantee check.
Review: https://reviews.apache.org/r/42739
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/23d009ee
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/23d009ee
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/23d009ee
Branch: refs/heads/master
Commit: 23d009ee1799f685af406945241dce6cf34014d2
Parents: 8f0e7fe
Author: Michael Park <mp...@apache.org>
Authored: Mon Jan 25 12:26:42 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Tue Jan 26 20:06:28 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 35 ++++++++---
src/tests/hierarchical_allocator_tests.cpp | 75 ++++++++++++++++++++++++
2 files changed, 101 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/23d009ee/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 02fe9fe..1e1c0b0 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1149,6 +1149,29 @@ void HierarchicalAllocatorProcess::allocate(
// TODO(vinod): Implement a smarter sorting algorithm.
std::random_shuffle(slaveIds.begin(), slaveIds.end());
+ // Returns the __amount__ of resources allocated to a quota role. Since we
+ // account for reservations and persistent volumes toward quota, we strip
+ // reservation and persistent volume related information for comparability.
+ // The result is used to determine whether a role's quota is satisfied, and
+ // also to determine how many resources the role would need in order to meet
+ // its quota.
+ //
+ // NOTE: Revocable resources are excluded in `quotaRoleSorter`.
+ auto getQuotaRoleAllocatedResources = [this](const std::string& role) {
+ CHECK(quotas.contains(role));
+
+ Resources resources = quotaRoleSorter->allocationScalars(role);
+
+ // Strip the reservation and persistent volume info.
+ foreach (Resource& resource, resources) {
+ resource.set_role("*");
+ resource.clear_reservation();
+ resource.clear_disk();
+ }
+
+ return resources;
+ };
+
// Quota comes first and fair share second. Here we process only those
// roles, for which quota is set (quota'ed roles). Such roles form a
// special allocation group with a dedicated sorter.
@@ -1162,14 +1185,8 @@ void HierarchicalAllocatorProcess::allocate(
continue;
}
- // Summing up resources is fine because quota is only for scalar
- // resources.
- //
- // NOTE: Revocable resources are excluded in `quotaRoleSorter`.
- //
- // TODO(alexr): Consider including dynamically reserved resources.
- Resources roleConsumedResources =
- Resources::sum(quotaRoleSorter->allocation(role));
+ // Get the total amount of resources allocated to a quota role.
+ 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.
@@ -1258,7 +1275,7 @@ void HierarchicalAllocatorProcess::allocate(
// Compute the amount of quota that the role does not have allocated.
//
// NOTE: Revocable resources are excluded in `quotaRoleSorter`.
- Resources allocated = Resources::sum(quotaRoleSorter->allocation(name));
+ Resources allocated = getQuotaRoleAllocatedResources(name);
const Resources required = quota.info.guarantee();
unallocatedQuotaResources += (required - allocated);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/23d009ee/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 5777dd7..e4c26ab 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -2244,6 +2244,81 @@ TEST_F(HierarchicalAllocatorTest, MultiQuotaWithFrameworks)
}
+// This tests that reserved resources are accounted for in the role's quota.
+TEST_F(HierarchicalAllocatorTest, ReservationWithinQuota)
+{
+ // Pausing the clock is not necessary, but ensures that the test
+ // doesn't rely on the periodic allocation in the allocator, which
+ // would slow down the test.
+ Clock::pause();
+
+ const string QUOTA_ROLE{"quota-role"};
+ const string NON_QUOTA_ROLE{"non-quota-role"};
+
+ initialize();
+
+ FrameworkInfo framework1 = createFrameworkInfo(QUOTA_ROLE);
+ FrameworkInfo framework2 = createFrameworkInfo(NON_QUOTA_ROLE);
+
+ const Quota quota = createQuota(QUOTA_ROLE, "cpus:2;mem:256");
+
+ // Notify allocator of agents, frameworks, quota and current allocations.
+ allocator->setQuota(QUOTA_ROLE, quota);
+
+ allocator->addFramework(
+ framework1.id(),
+ framework1,
+ hashmap<SlaveID, Resources>());
+
+ allocator->addFramework(
+ framework2.id(),
+ framework2,
+ hashmap<SlaveID, Resources>());
+
+ // Process all triggered allocation events.
+ //
+ // NOTE: No allocations happen because there are no resources to allocate.
+ Clock::settle();
+
+ // Some resources on `agent1` are now being used by `framework1` as part
+ // of its role quota. `framework2` will be offered the rest of `agent1`'s
+ // resources since `framework1`'s quota is satisfied, and `framework2` has
+ // no resources.
+ SlaveInfo agent1 = createSlaveInfo("cpus:8;mem(" + QUOTA_ROLE + "):256");
+ allocator->addSlave(
+ agent1.id(),
+ agent1,
+ None(),
+ agent1.resources(),
+ {std::make_pair(
+ framework1.id(),
+ // The `mem` portion is used to test that reserved resources are
+ // accounted for, and the `cpus` portion is allocated to show that
+ // the result of DRF would be different if `mem` was not accounted.
+ Resources::parse("cpus:2;mem(" + QUOTA_ROLE + "):256").get())});
+
+ Future<Allocation> allocation = allocations.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
+
+ EXPECT_EQ(Resources::parse("cpus:6").get(),
+ Resources::sum(allocation.get().resources));
+
+ // Since the reserved resources account towards the quota as well as being
+ // accounted for DRF, we expect these resources to also be allocated to
+ // `framework2`.
+ SlaveInfo agent2 = createSlaveInfo("cpus:4");
+ allocator->addSlave(agent2.id(), agent2, None(), agent2.resources(), {});
+
+ allocation = allocations.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
+
+ EXPECT_EQ(Resources::parse("cpus:4").get(),
+ Resources::sum(allocation.get().resources));
+}
+
+
// This test checks that if a framework suppresses offers, disconnects and
// reconnects again, it will start receiving resource offers again.
TEST_F(HierarchicalAllocatorTest, DeactivateAndReactivateFramework)
[2/7] mesos git commit: Included reserved resources in stage 1 of
allocation.
Posted by mp...@apache.org.
Included reserved resources in stage 1 of allocation.
Review: https://reviews.apache.org/r/42741
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/77f003a1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/77f003a1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/77f003a1
Branch: refs/heads/master
Commit: 77f003a11b6523dd915cc8200de526adb95c12c7
Parents: 23d009e
Author: Michael Park <mp...@apache.org>
Authored: Mon Jan 25 12:26:31 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Tue Jan 26 20:06:28 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/77f003a1/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 1e1c0b0..63de667 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1210,12 +1210,24 @@ void HierarchicalAllocatorProcess::allocate(
continue;
}
- // Quota is satisfied from the available unreserved non-revocable
- // resources on the agent.
- //
- // TODO(alexr): Consider adding dynamically reserved resources.
+ // Calculate the currently available resources on the slave.
Resources available = slaves[slaveId].total - slaves[slaveId].allocated;
- Resources resources = available.unreserved().nonRevocable();
+
+ // The resources we offer are the unreserved resources as well as the
+ // reserved resources for this particular role. This is necessary to
+ // ensure that we don't offer resources that are reserved for another
+ // role.
+ //
+ // NOTE: Currently, frameworks are allowed to have '*' role.
+ // Calling reserved('*') returns an empty Resources object.
+ //
+ // Quota is satisfied from the available non-revocable resources on the
+ // agent. It's important that we include reserved resources here since
+ // reserved resources are accounted towards the quota guarantee. If we
+ // were to rely on stage 2 to offer them out, they would not be checked
+ // against the quota guarantee.
+ Resources resources =
+ (available.unreserved() + available.reserved(role)).nonRevocable();
// NOTE: The resources may not be allocatable here, but they can be
// accepted by one of the frameworks during the second allocation
[5/7] mesos git commit: Added more getters to the `Sorter`.
Posted by mp...@apache.org.
Added more getters to the `Sorter`.
This patch adds the following getters:
- `total` (similar to `allocation`)
- `allocationScalars`
- `totalScalars`
The scalar versions are necessary to improve perf of the allocator.
Review: https://reviews.apache.org/r/42834
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8f0e7fe2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8f0e7fe2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8f0e7fe2
Branch: refs/heads/master
Commit: 8f0e7fe20498c28dddd8de2863b1b9a31fb804f4
Parents: 2339984
Author: Michael Park <mp...@apache.org>
Authored: Tue Jan 26 16:04:30 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Tue Jan 26 20:06:28 2016 -0800
----------------------------------------------------------------------
src/master/allocator/sorter/drf/sorter.cpp | 64 ++++++++++++++++---------
src/master/allocator/sorter/drf/sorter.hpp | 11 ++++-
src/master/allocator/sorter/sorter.hpp | 12 ++++-
3 files changed, 62 insertions(+), 25 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/8f0e7fe2/src/master/allocator/sorter/drf/sorter.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/sorter/drf/sorter.cpp b/src/master/allocator/sorter/drf/sorter.cpp
index 3a442f1..1f39f6d 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -129,14 +129,14 @@ void DRFSorter::update(
// Otherwise, we need to ensure we re-calculate the shares, as
// is being currently done, for safety.
- CHECK(total.resources[slaveId].contains(oldAllocation));
- CHECK(total.scalars.contains(oldAllocation.scalars()));
+ CHECK(total_.resources[slaveId].contains(oldAllocation));
+ CHECK(total_.scalars.contains(oldAllocation.scalars()));
- total.resources[slaveId] -= oldAllocation;
- total.resources[slaveId] += newAllocation;
+ total_.resources[slaveId] -= oldAllocation;
+ total_.resources[slaveId] += newAllocation;
- total.scalars -= oldAllocation.scalars();
- total.scalars += newAllocation.scalars();
+ total_.scalars -= oldAllocation.scalars();
+ total_.scalars += newAllocation.scalars();
CHECK(allocations[name].resources[slaveId].contains(oldAllocation));
CHECK(allocations[name].scalars.contains(oldAllocation.scalars()));
@@ -152,7 +152,7 @@ void DRFSorter::update(
}
-hashmap<SlaveID, Resources> DRFSorter::allocation(const string& name)
+const hashmap<SlaveID, Resources>& DRFSorter::allocation(const string& name)
{
CHECK(contains(name));
@@ -160,6 +160,14 @@ hashmap<SlaveID, Resources> DRFSorter::allocation(const string& name)
}
+const Resources& DRFSorter::allocationScalars(const string& name)
+{
+ CHECK(contains(name));
+
+ return allocations[name].scalars;
+}
+
+
hashmap<std::string, Resources> DRFSorter::allocation(const SlaveID& slaveId)
{
// TODO(jmlvanre): We can index the allocation by slaveId to make this faster.
@@ -192,6 +200,18 @@ Resources DRFSorter::allocation(const string& name, const SlaveID& slaveId)
}
+const hashmap<SlaveID, Resources>& DRFSorter::total() const
+{
+ return total_.resources;
+}
+
+
+const Resources& DRFSorter::totalScalars() const
+{
+ return total_.scalars;
+}
+
+
void DRFSorter::unallocated(
const string& name,
const SlaveID& slaveId,
@@ -213,8 +233,8 @@ void DRFSorter::unallocated(
void DRFSorter::add(const SlaveID& slaveId, const Resources& resources)
{
if (!resources.empty()) {
- total.resources[slaveId] += resources;
- total.scalars += resources.scalars();
+ total_.resources[slaveId] += resources;
+ total_.scalars += resources.scalars();
// We have to recalculate all shares when the total resources
// change, but we put it off until sort is called so that if
@@ -228,13 +248,13 @@ void DRFSorter::add(const SlaveID& slaveId, const Resources& resources)
void DRFSorter::remove(const SlaveID& slaveId, const Resources& resources)
{
if (!resources.empty()) {
- CHECK(total.resources.contains(slaveId));
+ CHECK(total_.resources.contains(slaveId));
- total.resources[slaveId] -= resources;
- total.scalars -= resources.scalars();
+ total_.resources[slaveId] -= resources;
+ total_.scalars -= resources.scalars();
- if (total.resources[slaveId].empty()) {
- total.resources.erase(slaveId);
+ if (total_.resources[slaveId].empty()) {
+ total_.resources.erase(slaveId);
}
dirty = true;
@@ -244,15 +264,15 @@ void DRFSorter::remove(const SlaveID& slaveId, const Resources& resources)
void DRFSorter::update(const SlaveID& slaveId, const Resources& resources)
{
- CHECK(total.scalars.contains(total.resources[slaveId].scalars()));
+ CHECK(total_.scalars.contains(total_.resources[slaveId].scalars()));
- total.scalars -= total.resources[slaveId].scalars();
- total.scalars += resources.scalars();
+ total_.scalars -= total_.resources[slaveId].scalars();
+ total_.scalars += resources.scalars();
- total.resources[slaveId] = resources;
+ total_.resources[slaveId] = resources;
- if (total.resources[slaveId].empty()) {
- total.resources.erase(slaveId);
+ if (total_.resources[slaveId].empty()) {
+ total_.resources.erase(slaveId);
}
dirty = true;
@@ -325,12 +345,12 @@ double DRFSorter::calculateShare(const string& name)
// currently does not take into account resources that are not
// scalars.
- foreach (const string& scalar, total.scalars.names()) {
+ foreach (const string& scalar, total_.scalars.names()) {
double _total = 0.0;
// NOTE: Scalar resources may be spread across multiple
// 'Resource' objects. E.g. persistent volumes.
- foreach (const Resource& resource, total.scalars.get(scalar)) {
+ foreach (const Resource& resource, total_.scalars.get(scalar)) {
CHECK_EQ(resource.type(), Value::SCALAR);
_total += resource.scalar().value();
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/8f0e7fe2/src/master/allocator/sorter/drf/sorter.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/sorter/drf/sorter.hpp b/src/master/allocator/sorter/drf/sorter.hpp
index 050896e..4669149 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -87,12 +87,19 @@ public:
const SlaveID& slaveId,
const Resources& resources);
- virtual hashmap<SlaveID, Resources> allocation(const std::string& name);
+ virtual const hashmap<SlaveID, Resources>& allocation(
+ const std::string& name);
+
+ virtual const Resources& allocationScalars(const std::string& name);
virtual hashmap<std::string, Resources> allocation(const SlaveID& slaveId);
virtual Resources allocation(const std::string& name, const SlaveID& slaveId);
+ virtual const hashmap<SlaveID, Resources>& total() const;
+
+ virtual const Resources& totalScalars() const;
+
virtual void add(const SlaveID& slaveId, const Resources& resources);
virtual void remove(const SlaveID& slaveId, const Resources& resources);
@@ -134,7 +141,7 @@ private:
// that to speed up the calculation of shares. See MESOS-2891 for
// the reasons why we want to do that.
Resources scalars;
- } total;
+ } total_;
// Allocation for a client.
struct Allocation {
http://git-wip-us.apache.org/repos/asf/mesos/blob/8f0e7fe2/src/master/allocator/sorter/sorter.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/sorter/sorter.hpp b/src/master/allocator/sorter/sorter.hpp
index 7be6b44..a0a779b 100644
--- a/src/master/allocator/sorter/sorter.hpp
+++ b/src/master/allocator/sorter/sorter.hpp
@@ -76,7 +76,11 @@ public:
const Resources& resources) = 0;
// Returns the resources that have been allocated to this client.
- virtual hashmap<SlaveID, Resources> allocation(const std::string& client) = 0;
+ virtual const hashmap<SlaveID, Resources>& allocation(
+ const std::string& client) = 0;
+
+ // Returns the sum of the scalar resources that are allocated to this client.
+ virtual const Resources& allocationScalars(const std::string& client) = 0;
// Returns the clients that have allocations on this slave.
virtual hashmap<std::string, Resources> allocation(
@@ -88,6 +92,12 @@ public:
const std::string& client,
const SlaveID& slaveId) = 0;
+ // Returns the total resources that are in this sorter.
+ virtual const hashmap<SlaveID, Resources>& total() const = 0;
+
+ // Returns the sum of the total scalar resources that are in this sorter.
+ virtual const Resources& totalScalars() const = 0;
+
// Add resources to the total pool of resources this
// Sorter should consider.
virtual void add(const SlaveID& slaveId, const Resources& resources) = 0;
[7/7] mesos git commit: Quota: Only used scalars for lay-away
calculations.
Posted by mp...@apache.org.
Quota: Only used scalars for lay-away calculations.
Also used cached values from updated sorter API.
Review: https://reviews.apache.org/r/42836/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2f9e91e0
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2f9e91e0
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2f9e91e0
Branch: refs/heads/master
Commit: 2f9e91e064848c4db039c75d2d76ad1a297c0ba2
Parents: 7f16187
Author: Joris Van Remoortere <jo...@gmail.com>
Authored: Tue Jan 26 20:07:56 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Tue Jan 26 20:07:56 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 30 ++++++++++++------------
1 file changed, 15 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2f9e91e0/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 29856e1..dbc77a2 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1264,20 +1264,17 @@ void HierarchicalAllocatorProcess::allocate(
}
}
- // Calculate how many resources (including revocable and reserved) 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.
+ // Calculate how many scalar resources (including revocable and reserved)
+ // 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.
//
- // NOTE: We use all active agents and not just those visible 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;
- foreachkey (const SlaveID& slaveId, slaves) {
- if (isWhitelisted(slaveId) && slaves[slaveId].activated) {
- remainingClusterResources +=
- slaves[slaveId].total - slaves[slaveId].allocated;
- }
+ // 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->totalScalars();
+ foreachkey (const string& role, activeRoles) {
+ remainingClusterResources -= roleSorter->allocationScalars(role);
}
// Frameworks in a quota'ed role may temporarily reject resources by
@@ -1287,6 +1284,7 @@ void HierarchicalAllocatorProcess::allocate(
// 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);
const Resources required = quota.info.guarantee();
unallocatedQuotaResources += (required - allocated);
@@ -1369,7 +1367,9 @@ void HierarchicalAllocatorProcess::allocate(
// 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`.
- if (!remainingClusterResources.contains(allocatedStage2 + resources)) {
+ const Resources scalarResources = resources.scalars();
+ if (!remainingClusterResources.contains(
+ allocatedStage2 + scalarResources)) {
continue;
}
@@ -1382,7 +1382,7 @@ void HierarchicalAllocatorProcess::allocate(
// NOTE: We may have already allocated some resources on the current
// agent as part of quota.
offerable[frameworkId][slaveId] += resources;
- allocatedStage2 += resources;
+ allocatedStage2 += scalarResources;
slaves[slaveId].allocated += resources;
frameworkSorters[role]->add(slaveId, resources);