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 2014/12/12 22:58:16 UTC
[6/6] mesos git commit: Fixed the two level sorting in the allocator.
Fixed the two level sorting in the allocator.
Review: https://reviews.apache.org/r/28824
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/87e299f3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/87e299f3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/87e299f3
Branch: refs/heads/master
Commit: 87e299f3ec628682f43a90cc310f6f32024c2905
Parents: df521b0
Author: Benjamin Mahler <be...@gmail.com>
Authored: Mon Dec 8 12:09:26 2014 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Fri Dec 12 13:43:38 2014 -0800
----------------------------------------------------------------------
src/master/hierarchical_allocator_process.hpp | 95 +++++++++++++---------
src/master/sorter.hpp | 3 +
src/tests/hierarchical_allocator_tests.cpp | 74 ++++++++++++++++-
3 files changed, 133 insertions(+), 39 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/87e299f3/src/master/hierarchical_allocator_process.hpp
----------------------------------------------------------------------
diff --git a/src/master/hierarchical_allocator_process.hpp b/src/master/hierarchical_allocator_process.hpp
index 94995c0..95fa520 100644
--- a/src/master/hierarchical_allocator_process.hpp
+++ b/src/master/hierarchical_allocator_process.hpp
@@ -197,8 +197,6 @@ protected:
// 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.
- // TODO(bmahler): The code is currently inconsistent in following
- // this specification, see MESOS-2176.
RoleSorter* roleSorter;
hashmap<std::string, FrameworkSorter*> frameworkSorters;
};
@@ -275,7 +273,11 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::initialize(
frameworkSorters[name] = new FrameworkSorter();
}
- VLOG(1) << "Initializing hierarchical allocator process";
+ if (roleSorter->count() == 0) {
+ LOG(ERROR) << "No roles specified, cannot allocate resources!";
+ }
+
+ VLOG(1) << "Initialized hierarchical allocator process";
delay(flags.allocation_interval, self(), &Self::batch);
}
@@ -297,8 +299,11 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::addFramework(
CHECK(!frameworkSorters[role]->contains(frameworkId.value()));
frameworkSorters[role]->add(frameworkId.value());
+ // TODO(bmahler): Validate that the reserved resources have the
+ // framework's role.
+
// Update the allocation to this framework.
- roleSorter->allocated(role, used);
+ roleSorter->allocated(role, used.unreserved());
frameworkSorters[role]->add(used);
frameworkSorters[role]->allocated(frameworkId.value(), used);
@@ -327,7 +332,8 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::removeFramework(
if (frameworkSorters[role]->contains(frameworkId.value())) {
Resources allocation =
frameworkSorters[role]->allocation(frameworkId.value());
- roleSorter->unallocated(role, allocation);
+
+ roleSorter->unallocated(role, allocation.unreserved());
frameworkSorters[role]->remove(allocation);
frameworkSorters[role]->remove(frameworkId.value());
}
@@ -403,6 +409,7 @@ Resources sum(const Iterable& resources)
} // namespace internal {
+
template <class RoleSorter, class FrameworkSorter>
void
HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::addSlave(
@@ -414,17 +421,20 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::addSlave(
CHECK(initialized);
CHECK(!slaves.contains(slaveId));
- roleSorter->add(total);
+ roleSorter->add(total.unreserved());
foreachpair (const FrameworkID& frameworkId,
- const Resources& resources,
+ const Resources& allocated,
used) {
if (frameworks.contains(frameworkId)) {
const std::string& role = frameworks[frameworkId].role;
- frameworkSorters[role]->add(resources);
- frameworkSorters[role]->allocated(frameworkId.value(), resources);
- roleSorter->allocated(role, resources);
+ // TODO(bmahler): Validate that the reserved resources have the
+ // framework's role.
+
+ roleSorter->allocated(role, allocated.unreserved());
+ frameworkSorters[role]->add(allocated);
+ frameworkSorters[role]->allocated(frameworkId.value(), allocated);
}
}
@@ -451,7 +461,13 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::removeSlave(
CHECK(initialized);
CHECK(slaves.contains(slaveId));
- roleSorter->remove(slaves[slaveId].total);
+ // TODO(bmahler): Per MESOS-621, this should remove the allocations
+ // that any frameworks have on this slave. Otherwise the caller may
+ // "leak" allocated resources accidentally if they forget to recover
+ // all the resources. Fixing this would require more information
+ // than what we currently track in the allocator.
+
+ roleSorter->remove(slaves[slaveId].total.unreserved());
slaves.erase(slaveId);
@@ -552,7 +568,7 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::recoverResources(
if (frameworkSorters[role]->contains(frameworkId.value())) {
frameworkSorters[role]->unallocated(frameworkId.value(), resources);
frameworkSorters[role]->remove(resources);
- roleSorter->unallocated(role, resources);
+ roleSorter->unallocated(role, resources.unreserved());
}
}
@@ -665,13 +681,13 @@ void
HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::allocate(
const SlaveID& slaveId)
{
- hashset<SlaveID> slaveIds;
- slaveIds.insert(slaveId);
-
Stopwatch stopwatch;
stopwatch.start();
- allocate(slaveIds);
+ // TODO(bmahler): Add initializer list constructor for hashset.
+ hashset<SlaveID> slaves;
+ slaves.insert(slaveId);
+ allocate(slaves);
VLOG(1) << "Performed allocation for slave " << slaveId << " in "
<< stopwatch.elapsed();
@@ -684,21 +700,22 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::allocate(
const hashset<SlaveID>& slaveIds_)
{
if (roleSorter->count() == 0) {
- VLOG(1) << "No roles to allocate resources!";
+ LOG(ERROR) << "No roles specified, cannot allocate resources!";
return;
}
- if (slaveIds_.empty()) {
- VLOG(1) << "No resources available to allocate!";
- return;
- }
+ // Compute the offerable resources, per framework:
+ // (1) For reserved resources on the slave, allocate these to a
+ // framework having the corresponding role.
+ // (2) For unreserved resources on the slave, allocate these
+ // to a framework of any role.
+ hashmap<FrameworkID, hashmap<SlaveID, Resources> > offerable;
// Randomize the order in which slaves' resources are allocated.
// TODO(vinod): Implement a smarter sorting algorithm.
std::vector<SlaveID> slaveIds(slaveIds_.begin(), slaveIds_.end());
std::random_shuffle(slaveIds.begin(), slaveIds.end());
- hashmap<FrameworkID, hashmap<SlaveID, Resources> > offerable;
foreach (const SlaveID& slaveId, slaveIds) {
// Don't send offers for non-whitelisted and deactivated slaves.
if (!isWhitelisted(slaveId) || !slaves[slaveId].activated) {
@@ -706,10 +723,10 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::allocate(
}
foreach (const std::string& role, roleSorter->sort()) {
- foreach (const std::string& frameworkIdValue,
+ foreach (const std::string& frameworkId_,
frameworkSorters[role]->sort()) {
FrameworkID frameworkId;
- frameworkId.set_value(frameworkIdValue);
+ frameworkId.set_value(frameworkId_);
Resources unreserved = slaves[slaveId].available.unreserved();
Resources resources = unreserved;
@@ -727,28 +744,32 @@ HierarchicalAllocatorProcess<RoleSorter, FrameworkSorter>::allocate(
continue;
}
- VLOG(1)
- << "Offering " << resources << " on slave " << slaveId
- << " to framework " << frameworkId;
+ VLOG(2) << "Allocating " << resources << " on slave " << slaveId
+ << " to framework " << frameworkId;
+ // Note that we perform "coarse-grained" allocation,
+ // meaning that we always allocate the entire remaining
+ // slave resources to a single framework.
offerable[frameworkId][slaveId] = resources;
-
- // Update slave resources.
slaves[slaveId].available -= resources;
- // Update the sorters.
- // We only count resources not reserved for this role
- // in the share the sorter considers.
- frameworkSorters[role]->add(unreserved);
- frameworkSorters[role]->allocated(frameworkIdValue, unreserved);
+ // Reserved resources are only accounted for in the framework
+ // sorter, since the reserved resources are not shared across
+ // roles.
+ frameworkSorters[role]->add(resources);
+ frameworkSorters[role]->allocated(frameworkId_, resources);
roleSorter->allocated(role, unreserved);
}
}
}
- // Now offer the resources to each framework.
- foreachkey (const FrameworkID& frameworkId, offerable) {
- offerCallback(frameworkId, offerable[frameworkId]);
+ if (offerable.empty()) {
+ VLOG(1) << "No resources available to allocate!";
+ } else {
+ // Now offer the resources to each framework.
+ foreachkey (const FrameworkID& frameworkId, offerable) {
+ offerCallback(frameworkId, offerable[frameworkId]);
+ }
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/87e299f3/src/master/sorter.hpp
----------------------------------------------------------------------
diff --git a/src/master/sorter.hpp b/src/master/sorter.hpp
index 818967f..0150f90 100644
--- a/src/master/sorter.hpp
+++ b/src/master/sorter.hpp
@@ -32,6 +32,9 @@ namespace allocator {
// Sorters implement the logic for determining the
// order in which users or frameworks should receive
// resource allocations.
+// TODO(bmahler): Templatize this on Client, so that callers can
+// don't need to do string conversion, e.g. FrameworkID, string role,
+// etc.
class Sorter
{
public:
http://git-wip-us.apache.org/repos/asf/mesos/blob/87e299f3/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index e429253..c43d352 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -164,7 +164,7 @@ Resources sum(const Iterable& iterable)
// TODO(bmahler): These tests were transformed directly from
// integration tests into unit tests. However, these tests
// should be simplified even further to each test a single
-// expected behavor, at which point we can have more tests
+// expected behavior, at which point we can have more tests
// that are each very small.
@@ -175,7 +175,7 @@ Resources sum(const Iterable& iterable)
// framework currently has the smallest share. Checking for proper DRF
// logic when resources are returned, frameworks exit, etc. is handled
// by SorterTest.DRFSorter.
-TEST_F(HierarchicalAllocatorTest, DRF)
+TEST_F(HierarchicalAllocatorTest, UnreservedDRF)
{
// Pausing the clock is not necessary, but ensures that the test
// doesn't rely on the periodic allocation in the allocator, which
@@ -294,6 +294,76 @@ TEST_F(HierarchicalAllocatorTest, DRF)
}
+// This test ensures that reserved resources do not affect the sharing
+// across roles. However, reserved resources should be shared fairly
+// *within* a role.
+TEST_F(HierarchicalAllocatorTest, ReservedDRF)
+{
+ // 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();
+
+ initialize({"role1", "role2"});
+
+ hashmap<FrameworkID, Resources> EMPTY;
+
+ SlaveInfo slave1 = createSlaveInfo(
+ "cpus:1;mem:512;disk:0;"
+ "cpus(role1):100;mem(role1):1024;disk(role1):0");
+ allocator->addSlave(slave1.id(), slave1, slave1.resources(), EMPTY);
+
+ // framework1 will be offered all of the resources.
+ FrameworkInfo framework1 = createFrameworkInfo("role1");
+ allocator->addFramework(framework1.id(), framework1, Resources());
+
+ Future<Allocation> allocation = queue.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
+ EXPECT_EQ(slave1.resources(), sum(allocation.get().resources.values()));
+
+ FrameworkInfo framework2 = createFrameworkInfo("role2");
+ allocator->addFramework(framework2.id(), framework2, Resources());
+
+ // framework2 will be allocated the new resoures.
+ SlaveInfo slave2 = createSlaveInfo("cpus:2;mem:512;disk:0");
+ allocator->addSlave(slave2.id(), slave2, slave2.resources(), EMPTY);
+
+ allocation = queue.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
+ EXPECT_EQ(slave2.resources(), sum(allocation.get().resources.values()));
+
+ // 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.
+ SlaveInfo slave3 = createSlaveInfo("cpus:2;mem:512;disk:0");
+ allocator->addSlave(slave3.id(), slave3, slave3.resources(), EMPTY);
+
+ allocation = queue.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
+ EXPECT_EQ(slave3.resources(), sum(allocation.get().resources.values()));
+
+ // Now add another framework in role1. Since the reserved resources
+ // should be allocated fairly between frameworks within a role, we
+ // expect framework3 to receive the next allocation of role1
+ // resources.
+ FrameworkInfo framework3 = createFrameworkInfo("role1");
+ allocator->addFramework(framework3.id(), framework3, Resources());
+
+ SlaveInfo slave4 = createSlaveInfo(
+ "cpus(role1):2;mem(role1):1024;disk(role1):0");
+ allocator->addSlave(slave4.id(), slave4, slave4.resources(), EMPTY);
+
+ allocation = queue.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework3.id(), allocation.get().frameworkId);
+ EXPECT_EQ(slave4.resources(), sum(allocation.get().resources.values()));
+}
+
+
// This test ensures that allocation is done per slave. This is done
// by having 2 slaves and 2 frameworks and making sure each framework
// gets only one slave's resources during an allocation.