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/07/04 15:13:24 UTC
[2/4] mesos git commit: Simplified `DRFSorter` to not track per-agent
total resources.
Simplified `DRFSorter` to not track per-agent total resources.
The DRFSorter previously kept the total resources at each agent, along
with the total quantity of resources in the cluster. The latter figure
is what most of the clients of the sorter care about. In the few places
where the previous coding was using the per-agent total resources, it is
relatively easy to adjust the code to remove this dependency.
As part of this change, remove `total()` and
`update(const SlaveID& slaveId, const Resources& resources)` from the
Sorter interface. The former was unused; for the latter, code that used
it can instead be rewritten to adjust the total resources in the cluster
by first removing the previous resources at a agent and then adding
the new resources.
Review: https://reviews.apache.org/r/49375/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5a63f2a5
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5a63f2a5
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5a63f2a5
Branch: refs/heads/master
Commit: 5a63f2a5d2a5fe0e5315a5b8f79b75b99a6a5893
Parents: bc714c0
Author: Neil Conway <ne...@gmail.com>
Authored: Mon Jul 4 15:37:24 2016 +0200
Committer: Michael Park <mp...@apache.org>
Committed: Mon Jul 4 15:37:24 2016 +0200
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 24 ++++++++++-----
src/master/allocator/sorter/drf/sorter.cpp | 39 ------------------------
src/master/allocator/sorter/drf/sorter.hpp | 12 ++------
src/master/allocator/sorter/sorter.hpp | 6 ----
src/tests/sorter_tests.cpp | 12 +++++---
5 files changed, 28 insertions(+), 65 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/5a63f2a5/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index eca949e..15eb76d 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -521,17 +521,23 @@ void HierarchicalAllocatorProcess::updateSlave(
// Check that all the oversubscribed resources are revocable.
CHECK_EQ(oversubscribed, oversubscribed.revocable());
+ const Resources oldRevocable = slaves[slaveId].total.revocable();
+
// Update the total resources.
// Remove the old oversubscribed resources from the total and then
// add the new estimate of oversubscribed resources.
slaves[slaveId].total = slaves[slaveId].total.nonRevocable() + oversubscribed;
- // Now, update the total resources in the role sorters.
- roleSorter->update(slaveId, slaves[slaveId].total);
+ // Update the total resources in the `roleSorter` by removing the
+ // previous oversubscribed resources and adding the new
+ // oversubscription estimate.
+ roleSorter->remove(slaveId, oldRevocable);
+ roleSorter->add(slaveId, oversubscribed);
- // See comment at `quotaRoleSorter` declaration regarding non-revocable.
- quotaRoleSorter->update(slaveId, slaves[slaveId].total.nonRevocable());
+ // NOTE: We do not need to update `quotaRoleSorter` because this
+ // function only changes the revocable resources on the slave, but
+ // the quota role sorter only manages non-revocable resources.
LOG(INFO) << "Agent " << slaveId << " (" << slaves[slaveId].hostname << ")"
<< " updated with oversubscribed resources " << oversubscribed
@@ -693,13 +699,17 @@ Future<Nothing> HierarchicalAllocatorProcess::updateAvailable(
Try<Resources> updatedTotal = slaves[slaveId].total.apply(operations);
CHECK_SOME(updatedTotal);
+ const Resources oldTotal = slaves[slaveId].total;
slaves[slaveId].total = updatedTotal.get();
- // Now, update the total resources in the role sorters.
- roleSorter->update(slaveId, slaves[slaveId].total);
+ // Now, update the total resources in the role sorters by removing
+ // the previous resources at this slave and adding the new resources.
+ roleSorter->remove(slaveId, oldTotal);
+ roleSorter->add(slaveId, updatedTotal.get());
// See comment at `quotaRoleSorter` declaration regarding non-revocable.
- quotaRoleSorter->update(slaveId, slaves[slaveId].total.nonRevocable());
+ quotaRoleSorter->remove(slaveId, oldTotal.nonRevocable());
+ quotaRoleSorter->add(slaveId, updatedTotal.get().nonRevocable());
return Nothing();
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/5a63f2a5/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 967290d..06176a3 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -191,12 +191,8 @@ void DRFSorter::update(
const Resources newAllocationQuantity =
newAllocation.createStrippedScalarQuantity();
- CHECK(total_.resources[slaveId].contains(oldAllocation));
CHECK(total_.scalarQuantities.contains(oldAllocationQuantity));
- total_.resources[slaveId] -= oldAllocation;
- total_.resources[slaveId] += newAllocation;
-
total_.scalarQuantities -= oldAllocationQuantity;
total_.scalarQuantities += newAllocationQuantity;
@@ -262,12 +258,6 @@ Resources DRFSorter::allocation(const string& name, const SlaveID& slaveId)
}
-const hashmap<SlaveID, Resources>& DRFSorter::total() const
-{
- return total_.resources;
-}
-
-
const Resources& DRFSorter::totalScalarQuantities() const
{
return total_.scalarQuantities;
@@ -296,7 +286,6 @@ void DRFSorter::unallocated(
void DRFSorter::add(const SlaveID& slaveId, const Resources& resources)
{
if (!resources.empty()) {
- total_.resources[slaveId] += resources;
total_.scalarQuantities += resources.createStrippedScalarQuantity();
// We have to recalculate all shares when the total resources
@@ -311,40 +300,12 @@ 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));
-
- total_.resources[slaveId] -= resources;
total_.scalarQuantities -= resources.createStrippedScalarQuantity();
-
- if (total_.resources[slaveId].empty()) {
- total_.resources.erase(slaveId);
- }
-
dirty = true;
}
}
-void DRFSorter::update(const SlaveID& slaveId, const Resources& resources)
-{
- const Resources oldSlaveQuantity =
- total_.resources[slaveId].createStrippedScalarQuantity();
-
- CHECK(total_.scalarQuantities.contains(oldSlaveQuantity));
-
- total_.scalarQuantities -= oldSlaveQuantity;
- total_.scalarQuantities += resources.createStrippedScalarQuantity();
-
- total_.resources[slaveId] = resources;
-
- if (total_.resources[slaveId].empty()) {
- total_.resources.erase(slaveId);
- }
-
- dirty = true;
-}
-
-
list<string> DRFSorter::sort()
{
if (dirty) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/5a63f2a5/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 0aa1a71..e29feeb 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -111,16 +111,12 @@ public:
virtual Resources allocation(const std::string& name, const SlaveID& slaveId);
- virtual const hashmap<SlaveID, Resources>& total() const;
-
virtual const Resources& totalScalarQuantities() const;
virtual void add(const SlaveID& slaveId, const Resources& resources);
virtual void remove(const SlaveID& slaveId, const Resources& resources);
- virtual void update(const SlaveID& slaveId, const Resources& resources);
-
virtual std::list<std::string> sort();
virtual bool contains(const std::string& name);
@@ -153,11 +149,9 @@ private:
// Total resources.
struct Total {
- hashmap<SlaveID, Resources> resources;
-
- // NOTE: Scalars can be safely aggregated across slaves. We keep
- // that to speed up the calculation of shares. See MESOS-2891 for
- // the reasons why we want to do that.
+ // NOTE: We do not need to track the total resources at each
+ // slave; instead, we can safely aggregate scalar resources across
+ // slaves, which substantially improves performance (MESOS-2891).
//
// NOTE: We omit information about dynamic reservations and persistent
// volumes here to enable resources to be aggregated across slaves
http://git-wip-us.apache.org/repos/asf/mesos/blob/5a63f2a5/src/master/allocator/sorter/sorter.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/sorter/sorter.hpp b/src/master/allocator/sorter/sorter.hpp
index 5ce6bb8..f5f0b08 100644
--- a/src/master/allocator/sorter/sorter.hpp
+++ b/src/master/allocator/sorter/sorter.hpp
@@ -116,9 +116,6 @@ 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 total scalar resource quantities in this sorter. This
// omits metadata about dynamic reservations and persistent volumes; see
// `Resources::createStrippedScalarQuantity`.
@@ -131,9 +128,6 @@ public:
// Remove resources from the total pool.
virtual void remove(const SlaveID& slaveId, const Resources& resources) = 0;
- // Updates the total pool of resources.
- virtual void update(const SlaveID& slaveId, const Resources& resources) = 0;
-
// Returns a list of all clients, in the order that they
// should be allocated to, according to this Sorter's policy.
virtual std::list<std::string> sort() = 0;
http://git-wip-us.apache.org/repos/asf/mesos/blob/5a63f2a5/src/tests/sorter_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 6fc5882..0a92134 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -379,8 +379,10 @@ TEST(SorterTest, UpdateTotal)
EXPECT_EQ("b", sorted.front());
EXPECT_EQ("a", sorted.back());
- // Update the total resources.
- sorter.update(slaveId, Resources::parse("cpus:100;mem:10").get());
+ // Update the total resources by removing the previous total and
+ // adding back the new total.
+ sorter.remove(slaveId, Resources::parse("cpus:10;mem:100").get());
+ sorter.add(slaveId, Resources::parse("cpus:100;mem:10").get());
// Now the dominant share of "a" is 0.1 (mem) and "b" is 0.2 (mem),
// which should change the sort order.
@@ -422,8 +424,10 @@ TEST(SorterTest, MultipleSlavesUpdateTotal)
EXPECT_EQ("b", sorted.front());
EXPECT_EQ("a", sorted.back());
- // Update the total resources of slaveA.
- sorter.update(slaveA, Resources::parse("cpus:95;mem:50").get());
+ // Update the total resources of slaveA by removing the previous
+ // total and adding the new total.
+ sorter.remove(slaveA, Resources::parse("cpus:5;mem:50").get());
+ sorter.add(slaveA, Resources::parse("cpus:95;mem:50").get());
// Now the dominant share of "a" is 0.02 (cpus) and "b" is 0.03
// (mem), which should change the sort order.