You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ne...@apache.org on 2017/05/24 21:44:56 UTC
[1/7] mesos git commit: Added sorter test for allocation queries
about inactive clients.
Repository: mesos
Updated Branches:
refs/heads/master 4faca73e9 -> a637df6d3
Added sorter test for allocation queries about inactive clients.
This behavior has not changed, but was not covered by the existing
tests.
Review: https://reviews.apache.org/r/59481/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7533901a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7533901a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7533901a
Branch: refs/heads/master
Commit: 7533901a3fd699adba7070e31d291026366e6431
Parents: 4faca73
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 23 10:36:12 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 24 14:39:58 2017 -0700
----------------------------------------------------------------------
src/tests/sorter_tests.cpp | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/7533901a/src/tests/sorter_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 2389664..ed66b9a 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -989,6 +989,43 @@ TEST(SorterTest, UpdateAllocationNestedClient)
}
+// This test checks that the sorter correctly reports allocation
+// information about inactive clients.
+TEST(SorterTest, AllocationForInactiveClient)
+{
+ DRFSorter sorter;
+
+ SlaveID slaveId;
+ slaveId.set_value("agentId");
+
+ sorter.add(slaveId, Resources::parse("cpus:10;mem:10").get());
+
+ sorter.add("a");
+ sorter.add("b");
+
+ // Leave client "a" inactive.
+ sorter.activate("b");
+
+ sorter.allocated("a", slaveId, Resources::parse("cpus:2;mem:2").get());
+ sorter.allocated("b", slaveId, Resources::parse("cpus:3;mem:3").get());
+
+ hashmap<string, Resources> clientAllocation = sorter.allocation(slaveId);
+ EXPECT_EQ(2u, clientAllocation.size());
+ EXPECT_EQ(Resources::parse("cpus:2;mem:2").get(), clientAllocation.at("a"));
+ EXPECT_EQ(Resources::parse("cpus:3;mem:3").get(), clientAllocation.at("b"));
+
+ hashmap<SlaveID, Resources> agentAllocation1 = sorter.allocation("a");
+ EXPECT_EQ(1u, agentAllocation1.size());
+ EXPECT_EQ(
+ Resources::parse("cpus:2;mem:2").get(), agentAllocation1.at(slaveId));
+
+ hashmap<SlaveID, Resources> agentAllocation2 = sorter.allocation("b");
+ EXPECT_EQ(1u, agentAllocation2.size());
+ EXPECT_EQ(
+ Resources::parse("cpus:3;mem:3").get(), agentAllocation2.at(slaveId));
+}
+
+
// We aggregate resources from multiple slaves into the sorter.
// Since non-scalar resources don't aggregate well across slaves,
// we need to keep track of the SlaveIDs of the resources. This
[6/7] mesos git commit: Introduced `DRFSorter::Node::isLeaf()`.
Posted by ne...@apache.org.
Introduced `DRFSorter::Node::isLeaf()`.
This helps clarify code that wants to distinguish between leaves and
internal nodes in the sorter.
Review: https://reviews.apache.org/r/59484/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fd58fa14
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fd58fa14
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fd58fa14
Branch: refs/heads/master
Commit: fd58fa149fad807232eb68fed2a4fc22c8a08f54
Parents: dcb0b7c
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 23 10:36:17 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 24 14:40:34 2017 -0700
----------------------------------------------------------------------
src/master/allocator/sorter/drf/sorter.cpp | 22 ++++++----------------
src/master/allocator/sorter/drf/sorter.hpp | 10 ++++++++++
2 files changed, 16 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/fd58fa14/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 74da5b1..1d8959d 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -99,13 +99,7 @@ void DRFSorter::add(const string& clientPath)
// leaf node into an internal node, we need to create an
// additional child node: `current` must have been associated with
// a client and clients must always be associated with leaf nodes.
- //
- // There are two exceptions: if `current` is the root node or it
- // was just created by the current `add()` call, it does not
- // correspond to a client, so we don't create an extra child.
- if (current->children.empty() &&
- current != root &&
- current != lastCreatedNode) {
+ if (current->isLeaf()) {
Node* parent = CHECK_NOTNULL(current->parent);
parent->removeChild(current);
@@ -122,9 +116,10 @@ void DRFSorter::add(const string& clientPath)
// `internal`.
current->name = ".";
current->parent = internal;
- internal->addChild(current);
current->path = strings::join("/", parent->path, current->name);
+ internal->addChild(current);
+
CHECK_EQ(internal->path, current->clientPath());
current = internal;
@@ -155,8 +150,7 @@ void DRFSorter::add(const string& clientPath)
}
// `current` is the newly created node associated with the last
- // element of the path. `current` should be an inactive node with no
- // children.
+ // element of the path. `current` should be an inactive leaf node.
CHECK(current->children.empty());
CHECK(current->kind == Node::INACTIVE_LEAF);
@@ -228,9 +222,7 @@ void DRFSorter::remove(const string& clientPath)
Node* child = *(current->children.begin());
if (child->name == ".") {
- CHECK(child->children.empty());
- CHECK(child->kind == Node::ACTIVE_LEAF ||
- child->kind == Node::INACTIVE_LEAF);
+ CHECK(child->isLeaf());
CHECK(clients.contains(current->path));
CHECK_EQ(child, clients.at(current->path));
@@ -578,9 +570,7 @@ DRFSorter::Node* DRFSorter::find(const string& clientPath) const
Node* client = client_.get();
- CHECK(client->kind == Node::ACTIVE_LEAF ||
- client->kind == Node::INACTIVE_LEAF);
- CHECK(client->children.empty());
+ CHECK(client->isLeaf());
return client;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/fd58fa14/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 f76d2f7..67700d5 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -267,6 +267,16 @@ struct DRFSorter::Node
return path;
}
+ bool isLeaf() const
+ {
+ if (kind == ACTIVE_LEAF || kind == INACTIVE_LEAF) {
+ CHECK(children.empty());
+ return true;
+ }
+
+ return false;
+ }
+
void removeChild(const Node* child)
{
auto it = std::find(children.begin(), children.end(), child);
[4/7] mesos git commit: Cleaned up allocator benchmark tests.
Posted by ne...@apache.org.
Cleaned up allocator benchmark tests.
Review: https://reviews.apache.org/r/59538
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8139ec0b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8139ec0b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8139ec0b
Branch: refs/heads/master
Commit: 8139ec0bd5c7daead3b7e4d05806898a48a6f62d
Parents: b7f33e4
Author: Neil Conway <ne...@gmail.com>
Authored: Wed May 24 13:19:55 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 24 14:40:21 2017 -0700
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 36 ++++++++++++++-----------
1 file changed, 21 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/8139ec0b/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 7e5ade2..eb2b647 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -4971,9 +4971,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, AddAndUpdateSlave)
// Add the slaves, use round-robin to choose which framework
// to allocate a slice of the slave's resources to.
for (size_t i = 0; i < slaves.size(); i++) {
- hashmap<FrameworkID, Resources> used;
-
- used[frameworks[i % frameworkCount].id()] = allocation;
+ hashmap<FrameworkID, Resources> used = {
+ {frameworks[i % frameworkCount].id(), allocation}
+ };
allocator->addSlave(
slaves[i].id(),
@@ -5096,10 +5096,12 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, DeclineOffers)
for (size_t i = 0; i < slaveCount; i++) {
slaves.push_back(createSlaveInfo(agentResources));
- // Add some used resources on each slave. Let's say there are 16 tasks, each
- // is allocated 1 cpu and a random port from the port range.
- hashmap<FrameworkID, Resources> used;
- used[frameworks[i % frameworkCount].id()] = allocation;
+ // Add some used resources on each slave. Let's say there are 16 tasks;
+ // each is allocated 1 cpu and a random port from the port range.
+ hashmap<FrameworkID, Resources> used = {
+ {frameworks[i % frameworkCount].id(), allocation}
+ };
+
allocator->addSlave(
slaves[i].id(),
slaves[i],
@@ -5289,8 +5291,10 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, ResourceLabels)
// Add some used resources on each slave. Let's say there are 16 tasks, each
// is allocated 1 cpu and a random port from the port range.
- hashmap<FrameworkID, Resources> used;
- used[frameworks[i % frameworkCount].id()] = _allocation;
+ hashmap<FrameworkID, Resources> used = {
+ {frameworks[i % frameworkCount].id(), _allocation}
+ };
+
allocator->addSlave(
slaves[i].id(),
slaves[i],
@@ -5424,8 +5428,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, SuppressOffers)
for (size_t i = 0; i < agentCount; i++) {
agents.push_back(createSlaveInfo(agentResources));
- hashmap<FrameworkID, Resources> used;
- used[frameworks[i % frameworkCount].id()] = allocation;
+ hashmap<FrameworkID, Resources> used = {
+ {frameworks[i % frameworkCount].id(), allocation}
+ };
allocator->addSlave(
agents[i].id(),
@@ -5454,7 +5459,7 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, SuppressOffers)
size_t allocationsCount = 5;
size_t suppressCount = 0;
- for (size_t i = 0; i < allocationsCount; ++i) {
+ for (size_t i = 0; i < allocationsCount; i++) {
// Recover resources with no filters because we want to test the
// effect of suppression alone.
foreach (const OfferedResources& offer, offers) {
@@ -5580,8 +5585,9 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, ExtremeSuppressOffers)
for (size_t i = 0; i < agentCount; i++) {
agents.push_back(createSlaveInfo(agentResources));
- hashmap<FrameworkID, Resources> used;
- used[frameworks[i % frameworkCount].id()] = allocation;
+ hashmap<FrameworkID, Resources> used = {
+ {frameworks[i % frameworkCount].id(), allocation}
+ };
allocator->addSlave(
agents[i].id(),
@@ -5617,7 +5623,7 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, ExtremeSuppressOffers)
allocator->suppressOffers(frameworks[i].id(), {});
}
- for (size_t i = 0; i < allocationsCount; ++i) {
+ for (size_t i = 0; i < allocationsCount; i++) {
// Recover resources with no filters because we want to test the
// effect of suppression alone.
foreach (const OfferedResources& offer, offers) {
[7/7] mesos git commit: Optimized sorter performance with many
inactive clients.
Posted by ne...@apache.org.
Optimized sorter performance with many inactive clients.
Unlike in Mesos <= 1.2, the sorter now stores inactive clients in the
same data structure used to store active clients. This resulted in a
significant performance regression when the vast majority of sorter
clients are inactive: when sorting clients and producing the sorted
order, we iterate over ALL clients (active and inactive), which could be
much slower than the old active-only implementation.
This commit revises the sorter to ensure that inactive leaf nodes are
always stored at the end of their parent's list of child nodes. This
allows the sorter to stop early (at the first inactive leaf) when
iterating over a node's children, if we're only interested in applying
an operation to each active leaf or internal node. This change fixes the
observed performance regression relative to Mesos 1.2.0.
Review: https://reviews.apache.org/r/59355/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a637df6d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a637df6d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a637df6d
Branch: refs/heads/master
Commit: a637df6d3807ab13ce6f426a03cb4396ffa453e6
Parents: fd58fa1
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 23 10:36:18 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 24 14:40:39 2017 -0700
----------------------------------------------------------------------
src/master/allocator/sorter/drf/sorter.cpp | 98 +++++++++++++++++++++----
src/master/allocator/sorter/drf/sorter.hpp | 27 ++++++-
2 files changed, 110 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a637df6d/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 1d8959d..ecc5586 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -147,6 +147,14 @@ void DRFSorter::add(const string& clientPath)
// If we created `current` in the loop above, it was marked an
// `INTERNAL` node. It should actually be an inactive leaf node.
current->kind = Node::INACTIVE_LEAF;
+
+ // `current` has changed from an internal node to an inactive
+ // leaf, so remove and re-add it to its parent. This moves it to
+ // the end of the parent's list of children.
+ CHECK_NOTNULL(current->parent);
+
+ current->parent->removeChild(current);
+ current->parent->addChild(current);
}
// `current` is the newly created node associated with the last
@@ -229,6 +237,16 @@ void DRFSorter::remove(const string& clientPath)
current->kind = child->kind;
current->removeChild(child);
+ // `current` has changed kind (from `INTERNAL` to a leaf,
+ // which might be active or inactive). Hence we might need to
+ // change its position in the `children` list.
+ if (current->kind == Node::INTERNAL) {
+ CHECK_NOTNULL(current->parent);
+
+ current->parent->removeChild(current);
+ current->parent->addChild(current);
+ }
+
clients[current->path] = current;
delete child;
@@ -250,14 +268,41 @@ void DRFSorter::remove(const string& clientPath)
void DRFSorter::activate(const string& clientPath)
{
Node* client = CHECK_NOTNULL(find(clientPath));
- client->kind = Node::ACTIVE_LEAF;
+
+ if (client->kind == Node::INACTIVE_LEAF) {
+ client->kind = Node::ACTIVE_LEAF;
+
+ // `client` has been activated, so move it to the beginning of its
+ // parent's list of children. We mark the tree dirty, so that the
+ // client's share is updated correctly and it is sorted properly.
+ //
+ // TODO(neilc): We could instead calculate share here and insert
+ // the client into the appropriate place here, which would avoid
+ // dirtying the whole tree.
+ CHECK_NOTNULL(client->parent);
+
+ client->parent->removeChild(client);
+ client->parent->addChild(client);
+
+ dirty = true;
+ }
}
void DRFSorter::deactivate(const string& clientPath)
{
Node* client = CHECK_NOTNULL(find(clientPath));
- client->kind = Node::INACTIVE_LEAF;
+
+ if (client->kind == Node::ACTIVE_LEAF) {
+ client->kind = Node::INACTIVE_LEAF;
+
+ // `client` has been deactivated, so move it to the end of its
+ // parent's list of children.
+ CHECK_NOTNULL(client->parent);
+
+ client->parent->removeChild(client);
+ client->parent->addChild(client);
+ }
}
@@ -467,16 +512,33 @@ vector<string> DRFSorter::sort()
{
if (dirty) {
std::function<void (Node*)> sortTree = [this, &sortTree](Node* node) {
- foreach (Node* child, node->children) {
+ // Inactive leaves are always stored at the end of the
+ // `children` vector; this means that as soon as we see an
+ // inactive leaf, we can stop calculating shares, and we only
+ // need to sort the prefix of the vector before that point.
+ auto childIter = node->children.begin();
+
+ while (childIter != node->children.end()) {
+ Node* child = *childIter;
+
+ if (child->kind == Node::INACTIVE_LEAF) {
+ break;
+ }
+
child->share = calculateShare(child);
+ ++childIter;
}
std::sort(node->children.begin(),
- node->children.end(),
+ childIter,
DRFSorter::Node::compareDRF);
foreach (Node* child, node->children) {
- sortTree(child);
+ if (child->kind == Node::INTERNAL) {
+ sortTree(child);
+ } else if (child->kind == Node::INACTIVE_LEAF) {
+ break;
+ }
}
};
@@ -485,18 +547,28 @@ vector<string> DRFSorter::sort()
dirty = false;
}
- // Return the leaf nodes in the tree. The children of each node are
- // already sorted in DRF order.
+ // Return all active leaves in the tree via pre-order traversal.
+ // The children of each node are already sorted in DRF order, with
+ // inactive leaves sorted after active leaves and internal nodes.
vector<string> result;
std::function<void (const Node*)> listClients =
[&listClients, &result](const Node* node) {
- if (node->kind == Node::ACTIVE_LEAF) {
- result.push_back(node->clientPath());
- }
-
- foreach (Node* child, node->children) {
- listClients(child);
+ foreach (const Node* child, node->children) {
+ switch (child->kind) {
+ case Node::ACTIVE_LEAF:
+ result.push_back(child->clientPath());
+ break;
+
+ case Node::INACTIVE_LEAF:
+ // As soon as we see the first inactive leaf, we can stop
+ // iterating over the current node's list of children.
+ return;
+
+ case Node::INTERNAL:
+ listClients(child);
+ break;
+ }
}
};
http://git-wip-us.apache.org/repos/asf/mesos/blob/a637df6d/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 67700d5..77e52de 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -125,7 +125,7 @@ private:
// Resources (by name) that will be excluded from fair sharing.
Option<std::set<std::string>> fairnessExcludeResourceNames;
- // If true, sort() will recalculate all shares.
+ // If true, sort() will recalculate all shares and resort the tree.
bool dirty = false;
// The root node in the sorter tree.
@@ -246,6 +246,19 @@ struct DRFSorter::Node
Kind kind;
Node* parent;
+
+ // Pointers to the child nodes. `children` is only non-empty if
+ // `kind` is INTERNAL_NODE. Two ordering invariants are maintained
+ // on the `children` vector:
+ //
+ // (1) All inactive leaves are stored at the end of the vector; that
+ // is, each `children` vector consists of zero or more active leaves
+ // and internal nodes, followed by zero or more inactive leaves. This
+ // means that code that only wants to iterate over active children
+ // can stop when the first inactive leaf is observed.
+ //
+ // (2) If the tree is not dirty, the active leaves and internal
+ // nodes are kept sorted by DRF share.
std::vector<Node*> children;
// If this node represents a sorter client, this returns the path of
@@ -279,6 +292,7 @@ struct DRFSorter::Node
void removeChild(const Node* child)
{
+ // Sanity check: ensure we are removing an extant node.
auto it = std::find(children.begin(), children.end(), child);
CHECK(it != children.end());
@@ -287,10 +301,19 @@ struct DRFSorter::Node
void addChild(Node* child)
{
+ // Sanity check: don't allow duplicates to be inserted.
auto it = std::find(children.begin(), children.end(), child);
CHECK(it == children.end());
- children.push_back(child);
+ // If we're inserting an inactive leaf, place it at the end of the
+ // `children` vector; otherwise, place it at the beginning. This
+ // maintains ordering invariant (1) above. It is up to the caller
+ // to maintain invariant (2) -- e.g., by marking the tree dirty.
+ if (child->kind == INACTIVE_LEAF) {
+ children.push_back(child);
+ } else {
+ children.insert(children.begin(), child);
+ }
}
// Allocation for a node.
[3/7] mesos git commit: Added benchmark for allocator perf with many
suppressed frameworks.
Posted by ne...@apache.org.
Added benchmark for allocator perf with many suppressed frameworks.
This covers the case where the vast majority (99%) of frameworks have
suppressed offers.
Review: https://reviews.apache.org/r/59383/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b7f33e47
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b7f33e47
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b7f33e47
Branch: refs/heads/master
Commit: b7f33e478793730d2334dbc92cff5c8ce558cfc7
Parents: 89e7bf5
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 23 10:36:14 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 24 14:40:13 2017 -0700
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 150 ++++++++++++++++++++++++
1 file changed, 150 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b7f33e47/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index f911110..7e5ade2 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -5501,6 +5501,156 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, SuppressOffers)
}
+// This benchmark measures allocator performance when almost all
+// frameworks are suppressed.
+TEST_P(HierarchicalAllocator_BENCHMARK_Test, ExtremeSuppressOffers)
+{
+ size_t agentCount = std::get<0>(GetParam());
+ size_t frameworkCount = std::get<1>(GetParam());
+
+ // Pause the clock because we want to manually drive the allocations.
+ Clock::pause();
+
+ struct OfferedResources
+ {
+ FrameworkID frameworkId;
+ SlaveID slaveId;
+ Resources resources;
+ };
+
+ vector<OfferedResources> offers;
+
+ auto offerCallback = [&offers](
+ const FrameworkID& frameworkId,
+ const hashmap<string, hashmap<SlaveID, Resources>>& resources_)
+ {
+ foreachkey (const string& role, resources_) {
+ foreachpair (const SlaveID& slaveId,
+ const Resources& resources,
+ resources_.at(role)) {
+ offers.push_back(OfferedResources{frameworkId, slaveId, resources});
+ }
+ }
+ };
+
+ cout << "Using " << agentCount << " agents and "
+ << frameworkCount << " frameworks" << endl;
+
+ master::Flags flags;
+ initialize(flags, offerCallback);
+
+ vector<FrameworkInfo> frameworks;
+ frameworks.reserve(frameworkCount);
+
+ Stopwatch watch;
+ watch.start();
+
+ for (size_t i = 0; i < frameworkCount; i++) {
+ frameworks.push_back(createFrameworkInfo({"*"}));
+ allocator->addFramework(frameworks[i].id(), frameworks[i], {}, true);
+ }
+
+ // Wait for all the `addFramework` operations to be processed.
+ Clock::settle();
+
+ watch.stop();
+
+ cout << "Added " << frameworkCount << " frameworks"
+ << " in " << watch.elapsed() << endl;
+
+ vector<SlaveInfo> agents;
+ agents.reserve(agentCount);
+
+ const Resources agentResources = Resources::parse(
+ "cpus:24;mem:4096;disk:4096;ports:[31000-32000]").get();
+
+ // Each agent has a portion of its resources allocated to a single
+ // framework. We round-robin through the frameworks when allocating.
+ Resources allocation = Resources::parse("cpus:16;mem:1024;disk:1024").get();
+
+ Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 32000), 16);
+ ASSERT_SOME(ranges);
+ ASSERT_EQ(16, ranges->range_size());
+
+ allocation += createPorts(ranges.get());
+ allocation.allocate("*");
+
+ watch.start();
+
+ for (size_t i = 0; i < agentCount; i++) {
+ agents.push_back(createSlaveInfo(agentResources));
+
+ hashmap<FrameworkID, Resources> used;
+ used[frameworks[i % frameworkCount].id()] = allocation;
+
+ allocator->addSlave(
+ agents[i].id(),
+ agents[i],
+ AGENT_CAPABILITIES(),
+ None(),
+ agents[i].resources(),
+ used);
+ }
+
+ // Wait for all the `addSlave` operations to be processed.
+ Clock::settle();
+
+ watch.stop();
+
+ cout << "Added " << agentCount << " agents"
+ << " in " << watch.elapsed() << endl;
+
+ // Now perform allocations. Each time we trigger an allocation run, we
+ // increase the number of frameworks that are suppressing offers. To
+ // ensure the test can run in a timely manner, we always perform a
+ // fixed number of allocations.
+ //
+ // TODO(jjanco): Parameterize this test by allocationsCount, not an arbitrary
+ // number. Batching reduces loop size, lowering time to test completion.
+ size_t allocationsCount = 5;
+
+ // Suppress offers for 99% of frameworks.
+ size_t suppressCount = static_cast<size_t>(frameworkCount * 0.99);
+ CHECK(suppressCount < frameworkCount);
+
+ for (size_t i = 0; i < suppressCount; i++) {
+ allocator->suppressOffers(frameworks[i].id(), {});
+ }
+
+ for (size_t i = 0; i < allocationsCount; ++i) {
+ // Recover resources with no filters because we want to test the
+ // effect of suppression alone.
+ foreach (const OfferedResources& offer, offers) {
+ allocator->recoverResources(
+ offer.frameworkId,
+ offer.slaveId,
+ offer.resources,
+ None());
+ }
+
+ // Wait for all declined offers to be processed.
+ Clock::settle();
+ offers.clear();
+
+ watch.start();
+
+ // Advance the clock and trigger a batch allocation.
+ Clock::advance(flags.allocation_interval);
+ Clock::settle();
+
+ watch.stop();
+
+ cout << "allocate() took " << watch.elapsed()
+ << " to make " << offers.size() << " offers with "
+ << suppressCount << " out of "
+ << frameworkCount << " frameworks suppressing offers"
+ << endl;
+ }
+
+ Clock::resume();
+}
+
+
// Measures the processing time required for the allocator metrics.
//
// TODO(bmahler): Add allocations to this benchmark.
[2/7] mesos git commit: Added a new sorter test case,
`HierarchicalIterationOrder`.
Posted by ne...@apache.org.
Added a new sorter test case, `HierarchicalIterationOrder`.
This behavior is covered by existing test cases to an extent, but some
additional test coverage seems warranted.
Review: https://reviews.apache.org/r/59482/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/89e7bf54
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/89e7bf54
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/89e7bf54
Branch: refs/heads/master
Commit: 89e7bf548c87c53a3e49fb6d930ed0d49d08affc
Parents: 7533901
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 23 10:36:13 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 24 14:40:08 2017 -0700
----------------------------------------------------------------------
src/tests/sorter_tests.cpp | 48 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/89e7bf54/src/tests/sorter_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index ed66b9a..6ca724d 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -605,6 +605,54 @@ TEST(SorterTest, HierarchicalAllocation)
}
+// This test checks that the sorted list of clients returned by the
+// sorter iterates over the client tree in the correct order.
+TEST(SorterTest, HierarchicalIterationOrder)
+{
+ DRFSorter sorter;
+
+ SlaveID slaveId;
+ slaveId.set_value("agentId");
+
+ Resources totalResources = Resources::parse("cpus:100;mem:100").get();
+ sorter.add(slaveId, totalResources);
+
+ sorter.add("a/b");
+ sorter.add("c");
+ sorter.add("d");
+ sorter.add("d/e");
+
+ sorter.activate("a/b");
+ sorter.activate("c");
+ sorter.activate("d");
+ sorter.activate("d/e");
+
+ // Shares: a/b = 0, c = 0, d = 0, d/e = 0
+ EXPECT_EQ(vector<string>({"a/b", "c", "d", "d/e"}), sorter.sort());
+
+ Resources cResources = Resources::parse("cpus:8;mem:8").get();
+ sorter.allocated("c", slaveId, cResources);
+
+ // Shares: a/b = 0, d = 0, d/e = 0, c = 0.08.
+ EXPECT_EQ(vector<string>({"a/b", "d", "d/e", "c"}), sorter.sort());
+
+ Resources dResources = Resources::parse("cpus:3;mem:3").get();
+ sorter.allocated("d", slaveId, dResources);
+
+ Resources deResources = Resources::parse("cpus:2;mem:2").get();
+ sorter.allocated("d/e", slaveId, deResources);
+
+ // Shares: a/b = 0, d/e = 0.02, d = 0.03, c = 0.08.
+ EXPECT_EQ(vector<string>({"a/b", "d/e", "d", "c"}), sorter.sort());
+
+ Resources abResources = Resources::parse("cpus:6;mem:6").get();
+ sorter.allocated("a/b", slaveId, abResources);
+
+ // Shares: d/e = 0.02, d = 0.03, a/b = 0.06, c = 0.08.
+ EXPECT_EQ(vector<string>({"d/e", "d", "a/b", "c"}), sorter.sort());
+}
+
+
// This test checks what happens when a new sorter client is added as
// a child of what was previously a leaf node.
TEST(SorterTest, AddChildToLeaf)
[5/7] mesos git commit: Replaced the sorter's notion of "activation"
with a three-valued enum.
Posted by ne...@apache.org.
Replaced the sorter's notion of "activation" with a three-valued enum.
Conceptually, a node in the sorter tree can either be an internal node
or a leaf node. Furthermore, leaf nodes can either be active or inactive
(internal nodes do not have a concept of "activation").
We previously represented this situation with a single boolean,
`active`. The boolean was true for active leaves, and false for inactive
leaves and internal nodes. Whether a node was an internal node was
determined by checking the number of children it has.
This commit replaces `active` with a three-valued enum named `kind`,
which can take on the values `ACTIVE_LEAF`, `INACTIVE_LEAF`, or
`INTERNAL`. This enforces the idea that internal nodes do not have a
notion of "activation", and more explicitly distinguishes between leaf
and internal nodes.
Review: https://reviews.apache.org/r/59483/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/dcb0b7ce
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/dcb0b7ce
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/dcb0b7ce
Branch: refs/heads/master
Commit: dcb0b7ce95309be64886c7ee36413c3665cfef9b
Parents: 8139ec0
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 23 10:36:16 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 24 14:40:28 2017 -0700
----------------------------------------------------------------------
src/master/allocator/sorter/drf/sorter.cpp | 40 +++++++++++++++++--------
src/master/allocator/sorter/drf/sorter.hpp | 22 +++++++++-----
2 files changed, 42 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/dcb0b7ce/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 26b77f5..74da5b1 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -45,13 +45,13 @@ namespace allocator {
DRFSorter::DRFSorter()
- : root(new Node("", nullptr)) {}
+ : root(new Node("", Node::INTERNAL, nullptr)) {}
DRFSorter::DRFSorter(
const UPID& allocator,
const string& metricsPrefix)
- : root(new Node("", nullptr)),
+ : root(new Node("", Node::INTERNAL, nullptr)),
metrics(Metrics(allocator, *this, metricsPrefix)) {}
@@ -112,7 +112,7 @@ void DRFSorter::add(const string& clientPath)
// Create a node under `parent`. This internal node will take
// the place of `current` in the tree.
- Node* internal = new Node(current->name, parent);
+ Node* internal = new Node(current->name, Node::INTERNAL, parent);
parent->addChild(internal);
internal->allocation = current->allocation;
@@ -131,28 +131,34 @@ void DRFSorter::add(const string& clientPath)
}
// Now actually add a new child to `current`.
- Node* newChild = new Node(element, current);
+ Node* newChild = new Node(element, Node::INTERNAL, current);
current->addChild(newChild);
current = newChild;
lastCreatedNode = newChild;
}
+ CHECK(current->kind == Node::INTERNAL);
+
// `current` is the node associated with the last element of the
// path. If we didn't add `current` to the tree above, create a leaf
// node now. For example, if the tree contains "a/b" and we add a
// new client "a", we want to create a new leaf node "a/." here.
if (current != lastCreatedNode) {
- Node* newChild = new Node(".", current);
+ Node* newChild = new Node(".", Node::INACTIVE_LEAF, current);
current->addChild(newChild);
current = newChild;
+ } else {
+ // If we created `current` in the loop above, it was marked an
+ // `INTERNAL` node. It should actually be an inactive leaf node.
+ current->kind = Node::INACTIVE_LEAF;
}
// `current` is the newly created node associated with the last
// element of the path. `current` should be an inactive node with no
// children.
CHECK(current->children.empty());
- CHECK(!current->active);
+ CHECK(current->kind == Node::INACTIVE_LEAF);
// Add a new entry to the lookup table. The full path of the newly
// added client should not already exist in `clients`.
@@ -223,10 +229,12 @@ void DRFSorter::remove(const string& clientPath)
if (child->name == ".") {
CHECK(child->children.empty());
+ CHECK(child->kind == Node::ACTIVE_LEAF ||
+ child->kind == Node::INACTIVE_LEAF);
CHECK(clients.contains(current->path));
CHECK_EQ(child, clients.at(current->path));
- current->active = child->active;
+ current->kind = child->kind;
current->removeChild(child);
clients[current->path] = current;
@@ -250,14 +258,14 @@ void DRFSorter::remove(const string& clientPath)
void DRFSorter::activate(const string& clientPath)
{
Node* client = CHECK_NOTNULL(find(clientPath));
- client->active = true;
+ client->kind = Node::ACTIVE_LEAF;
}
void DRFSorter::deactivate(const string& clientPath)
{
Node* client = CHECK_NOTNULL(find(clientPath));
- client->active = false;
+ client->kind = Node::INACTIVE_LEAF;
}
@@ -491,7 +499,7 @@ vector<string> DRFSorter::sort()
std::function<void (const Node*)> listClients =
[&listClients, &result](const Node* node) {
- if (node->active) {
+ if (node->kind == Node::ACTIVE_LEAF) {
result.push_back(node->clientPath());
}
@@ -562,13 +570,19 @@ double DRFSorter::findWeight(const Node* node) const
DRFSorter::Node* DRFSorter::find(const string& clientPath) const
{
- Option<Node*> client = clients.get(clientPath);
+ Option<Node*> client_ = clients.get(clientPath);
- if (client.isNone()) {
+ if (client_.isNone()) {
return nullptr;
}
- return client.get();
+ Node* client = client_.get();
+
+ CHECK(client->kind == Node::ACTIVE_LEAF ||
+ client->kind == Node::INACTIVE_LEAF);
+ CHECK(client->children.empty());
+
+ return client;
}
} // namespace allocator {
http://git-wip-us.apache.org/repos/asf/mesos/blob/dcb0b7ce/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 fee58d6..f76d2f7 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -195,8 +195,19 @@ private:
// and "c", and leaf nodes for the clients "a/b" and "c/d".
struct DRFSorter::Node
{
- Node(const std::string& _name, Node* _parent)
- : name(_name), share(0), active(false), parent(_parent)
+ // Indicates whether a node is an active leaf node, an inactive leaf
+ // node, or an internal node. Sorter clients always correspond to
+ // leaf nodes, and only leaf nodes can be activated or deactivated.
+ // The root node is always an "internal" node.
+ enum Kind
+ {
+ ACTIVE_LEAF,
+ INACTIVE_LEAF,
+ INTERNAL
+ };
+
+ Node(const std::string& _name, Kind _kind, Node* _parent)
+ : name(_name), share(0), kind(_kind), parent(_parent)
{
// Compute the node's path. Three cases:
//
@@ -232,11 +243,7 @@ struct DRFSorter::Node
double share;
- // True if this node represents an active sorter client. False if
- // this node represents an inactive sorter client or an internal node.
- //
- // TODO(neilc): Replace this with a three-valued enum?
- bool active;
+ Kind kind;
Node* parent;
std::vector<Node*> children;
@@ -253,6 +260,7 @@ struct DRFSorter::Node
std::string clientPath() const
{
if (name == ".") {
+ CHECK(kind == ACTIVE_LEAF || kind == INACTIVE_LEAF);
return CHECK_NOTNULL(parent)->path;
}