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 2017/12/04 22:33:06 UTC
[1/3] mesos git commit: Removed a stale TODO from before frameworks
could modify their roles.
Repository: mesos
Updated Branches:
refs/heads/master 3caca9716 -> 3c25233c2
Removed a stale TODO from before frameworks could modify their roles.
Now that frameworks can modify roles, it's possible for them to have
resources allocated to them for roles that they are no longer
subscribed to. We've already updated the code to handle this, so this
removes the stale TODOs from before this case was possible.
Review: https://reviews.apache.org/r/64237
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ecba7163
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ecba7163
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ecba7163
Branch: refs/heads/master
Commit: ecba7163191508888e5c02b07a8c44f26a75faff
Parents: 08f825b
Author: Benjamin Mahler <bm...@apache.org>
Authored: Thu Nov 30 16:12:37 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Dec 4 13:38:29 2017 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 5 -----
1 file changed, 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecba7163/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 715650e..9da6185 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -279,9 +279,6 @@ void HierarchicalAllocatorProcess::addFramework(
}
}
- // TODO(bmahler): Validate that the reserved resources have the
- // framework's role.
-
// Update the allocation for this framework.
foreachpair (const SlaveID& slaveId, const Resources& resources, used) {
if (!slaves.contains(slaveId)) {
@@ -2418,8 +2415,6 @@ void HierarchicalAllocatorProcess::trackAllocatedResources(
trackFrameworkUnderRole(frameworkId, role);
}
- // TODO(bmahler): Validate that the reserved resources have the
- // framework's role.
CHECK(roleSorter->contains(role));
CHECK(frameworkSorters.contains(role));
CHECK(frameworkSorters.at(role)->contains(frameworkId.value()));
[3/3] mesos git commit: Updated the allocator to track allocations
via a single code path.
Posted by bm...@apache.org.
Updated the allocator to track allocations via a single code path.
A helper was introduced for tracking allocated resources in the
sorters. This updates the code to allow the other two copies of
this code to use the function.
This also documents some of the addFramework/addSlave cases.
Review: https://reviews.apache.org/r/64238
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3c25233c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3c25233c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3c25233c
Branch: refs/heads/master
Commit: 3c25233c2ef456275c56373bd24321f264dd4fe0
Parents: ecba716
Author: Benjamin Mahler <bm...@apache.org>
Authored: Thu Nov 30 16:36:42 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Dec 4 14:32:38 2017 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 136 ++++++++++++++---------
src/master/allocator/mesos/hierarchical.hpp | 3 +-
2 files changed, 84 insertions(+), 55 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/3c25233c/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 9da6185..20b908c 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -281,11 +281,16 @@ void HierarchicalAllocatorProcess::addFramework(
// Update the allocation for this framework.
foreachpair (const SlaveID& slaveId, const Resources& resources, used) {
+ // TODO(bmahler): The master won't tell us about resources
+ // allocated to agents that have not yet been added, consider
+ // CHECKing this case.
if (!slaves.contains(slaveId)) {
continue;
}
- trackAllocatedResources(slaveId, {{frameworkId, resources}});
+ // The slave struct will already be aware of the allocated
+ // resources, so we only need to track them in the sorters.
+ trackAllocatedResources(slaveId, frameworkId, resources);
}
LOG(INFO) << "Added framework " << frameworkId;
@@ -504,13 +509,6 @@ void HierarchicalAllocatorProcess::addSlave(
CHECK(!slaves.contains(slaveId));
CHECK(!paused || expectedAgentCount.isSome());
- roleSorter->add(slaveId, total);
-
- // See comment at `quotaRoleSorter` declaration regarding non-revocable.
- quotaRoleSorter->add(slaveId, total.nonRevocable());
-
- trackAllocatedResources(slaveId, used);
-
slaves[slaveId] = Slave();
Slave& slave = slaves.at(slaveId);
@@ -531,6 +529,35 @@ void HierarchicalAllocatorProcess::addSlave(
slave.maintenance = Slave::Maintenance(unavailability.get());
}
+ roleSorter->add(slaveId, total);
+
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->add(slaveId, total.nonRevocable());
+
+ foreachpair (const FrameworkID& frameworkId,
+ const Resources& allocation,
+ used) {
+ // There are two cases here:
+ //
+ // (1) The framework has already been added to the allocator.
+ // In this case, we track the allocation in the sorters.
+ //
+ // (2) The framework has not yet been added to the allocator.
+ // The master will imminently add the framework using
+ // the `FrameworkInfo` recovered from the agent, and in
+ // the interim we do not track the resources allocated to
+ // this framework. This leaves a small window where the
+ // role sorting will under-account for the roles belonging
+ // to this framework.
+ //
+ // TODO(bmahler): Fix the issue outlined in (2).
+ if (!frameworks.contains(frameworkId)) {
+ continue;
+ }
+
+ trackAllocatedResources(slaveId, frameworkId, allocation);
+ }
+
// If we have just a number of recovered agents, we cannot distinguish
// between "old" agents from the registry and "new" ones joined after
// recovery has started. Because we do not persist enough information
@@ -634,7 +661,26 @@ void HierarchicalAllocatorProcess::addResourceProvider(
CHECK(initialized);
CHECK(slaves.contains(slaveId));
- trackAllocatedResources(slaveId, used);
+ foreachpair (const FrameworkID& frameworkId,
+ const Resources& allocation,
+ used) {
+ // There are two cases here:
+ //
+ // (1) The framework has already been added to the allocator.
+ // In this case, we track the allocation in the sorters.
+ //
+ // (2) The framework has not yet been added to the allocator.
+ // We do not track the resources allocated to this
+ // framework. This leaves a small window where the role
+ // sorting will under-account for the roles belonging
+ // to this framework. This case should never occur since
+ // the master will always add the framework first.
+ if (!frameworks.contains(frameworkId)) {
+ continue;
+ }
+
+ trackAllocatedResources(slaveId, frameworkId, allocation);
+ }
Slave& slave = slaves.at(slaveId);
updateSlaveTotal(slaveId, slave.total + total);
@@ -1654,14 +1700,7 @@ void HierarchicalAllocatorProcess::__allocate()
slave.allocated += resources;
- // Resources allocated as part of the quota count towards the
- // role's and the framework's fair share.
- //
- // NOTE: Revocable resources have already been excluded.
- frameworkSorter->add(slaveId, resources);
- frameworkSorter->allocated(frameworkId_, slaveId, resources);
- roleSorter->allocated(role, slaveId, resources);
- quotaRoleSorter->allocated(role, slaveId, resources);
+ trackAllocatedResources(slaveId, frameworkId, resources);
}
}
}
@@ -1872,15 +1911,7 @@ void HierarchicalAllocatorProcess::__allocate()
slave.allocated += resources;
- frameworkSorter->add(slaveId, resources);
- frameworkSorter->allocated(frameworkId_, slaveId, resources);
- roleSorter->allocated(role, slaveId, resources);
-
- if (quotas.contains(role)) {
- // See comment at `quotaRoleSorter` declaration regarding
- // non-revocable.
- quotaRoleSorter->allocated(role, slaveId, resources.nonRevocable());
- }
+ trackAllocatedResources(slaveId, frameworkId, resources);
}
}
}
@@ -2395,39 +2426,36 @@ bool HierarchicalAllocatorProcess::isRemoteSlave(const Slave& slave) const
void HierarchicalAllocatorProcess::trackAllocatedResources(
const SlaveID& slaveId,
- const hashmap<FrameworkID, Resources>& used)
+ const FrameworkID& frameworkId,
+ const Resources& allocated)
{
- // Update the allocation for each framework.
- foreachpair (const FrameworkID& frameworkId,
- const Resources& used_,
- used) {
- if (!frameworks.contains(frameworkId)) {
- continue;
- }
+ CHECK(slaves.contains(slaveId));
+ CHECK(frameworks.contains(frameworkId));
- foreachpair (const string& role,
- const Resources& allocated,
- used_.allocations()) {
- // The framework has resources allocated to this role but it may
- // or may not be subscribed to the role. Either way, we need to
- // track the framework under the role.
- if (!isFrameworkTrackedUnderRole(frameworkId, role)) {
- trackFrameworkUnderRole(frameworkId, role);
- }
+ // TODO(bmahler): Calling allocations() is expensive since it has
+ // to construct a map. Avoid this.
+ foreachpair (const string& role,
+ const Resources& allocation,
+ allocated.allocations()) {
+ // The framework has resources allocated to this role but it may
+ // or may not be subscribed to the role. Either way, we need to
+ // track the framework under the role.
+ if (!isFrameworkTrackedUnderRole(frameworkId, role)) {
+ trackFrameworkUnderRole(frameworkId, role);
+ }
- CHECK(roleSorter->contains(role));
- CHECK(frameworkSorters.contains(role));
- CHECK(frameworkSorters.at(role)->contains(frameworkId.value()));
+ CHECK(roleSorter->contains(role));
+ CHECK(frameworkSorters.contains(role));
+ CHECK(frameworkSorters.at(role)->contains(frameworkId.value()));
- roleSorter->allocated(role, slaveId, allocated);
- frameworkSorters.at(role)->add(slaveId, allocated);
- frameworkSorters.at(role)->allocated(
- frameworkId.value(), slaveId, allocated);
+ roleSorter->allocated(role, slaveId, allocation);
+ frameworkSorters.at(role)->add(slaveId, allocation);
+ frameworkSorters.at(role)->allocated(
+ frameworkId.value(), slaveId, allocation);
- if (quotas.contains(role)) {
- // See comment at `quotaRoleSorter` declaration regarding non-revocable.
- quotaRoleSorter->allocated(role, slaveId, allocated.nonRevocable());
- }
+ if (quotas.contains(role)) {
+ // See comment at `quotaRoleSorter` declaration regarding non-revocable.
+ quotaRoleSorter->allocated(role, slaveId, allocation.nonRevocable());
}
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/3c25233c/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 3c87dc7..41ffe17 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -550,7 +550,8 @@ private:
// Helper to track used resources on an agent.
void trackAllocatedResources(
const SlaveID& slaveId,
- const hashmap<FrameworkID, Resources>& used);
+ const FrameworkID& frameworkId,
+ const Resources& allocated);
};
[2/3] mesos git commit: Documented when
ReregisterSlaveMessage.frameworks was introduced.
Posted by bm...@apache.org.
Documented when ReregisterSlaveMessage.frameworks was introduced.
This field was populated by agents >= 1.0.0, per:
$ git tag --contains 3c96ca497ba2b0a733756f1af5e2dd4b4695d77a
Review: https://reviews.apache.org/r/64236
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/08f825b8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/08f825b8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/08f825b8
Branch: refs/heads/master
Commit: 08f825b8b8745c8a2169fd8c451f02d8670cfb05
Parents: 3caca97
Author: Benjamin Mahler <bm...@apache.org>
Authored: Thu Nov 30 16:07:55 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Dec 4 13:38:29 2017 -0800
----------------------------------------------------------------------
src/messages/messages.proto | 3 +++
1 file changed, 3 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/08f825b8/src/messages/messages.proto
----------------------------------------------------------------------
diff --git a/src/messages/messages.proto b/src/messages/messages.proto
index d7ef68f..2ab0fe8 100644
--- a/src/messages/messages.proto
+++ b/src/messages/messages.proto
@@ -514,7 +514,10 @@ message ReregisterSlaveMessage {
repeated ExecutorInfo executor_infos = 4;
repeated Task tasks = 3;
+
+ // The `FrameworkInfo`s are provided by agents that are >= 1.0.0.
repeated FrameworkInfo frameworks = 8;
+
repeated Archive.Framework completed_frameworks = 5;
// NOTE: This is a hack for the master to detect the agent's