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:56 UTC
[4/7] mesos git commit: Included reserved resources in the role
sorter for DRF.
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