Repository: mesos Updated Branches: refs/heads/master 1a9fd2a07 -> 6a8738f89
Allocator Performance: Exited early to avoid needless computation. Review: https://reviews.apache.org/r/43668/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6a8738f8 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6a8738f8 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6a8738f8 Branch: refs/heads/master Commit: 6a8738f89b01ac3ddd70c418c49f350e17fa5555 Parents: 1a9fd2a Author: Dario Rexin <dario.re...@me.com> Authored: Thu Mar 24 14:10:31 2016 +0100 Committer: Joris Van Remoortere <joris.van.remoort...@gmail.com> Committed: Thu Mar 24 14:12:41 2016 +0100 ---------------------------------------------------------------------- src/master/allocator/mesos/hierarchical.cpp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/6a8738f8/src/master/allocator/mesos/hierarchical.cpp ---------------------------------------------------------------------- diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp index 39a290d..a51c723 100644 --- a/src/master/allocator/mesos/hierarchical.cpp +++ b/src/master/allocator/mesos/hierarchical.cpp @@ -1281,11 +1281,16 @@ void HierarchicalAllocatorProcess::allocate( Resources resources = (available.unreserved() + available.reserved(role)).nonRevocable(); + // It is safe to break here, because all frameworks under a role would + // consider the same resources, so in case we don't have allocatable + // resources, we don't have to check for other frameworks under the + // same role. We only break out of the innermost loop, so the next step + // will use the same slaveId, but a different role. // NOTE: The resources may not be allocatable here, but they can be // accepted by one of the frameworks during the second allocation // stage. if (!allocatable(resources)) { - continue; + break; } // If the framework filters these resources, ignore. The unallocated @@ -1409,6 +1414,19 @@ void HierarchicalAllocatorProcess::allocate( resources += available.unreserved(); } + // It is safe to break here, because all frameworks under a role would + // consider the same resources, so in case we don't have allocatable + // resources, we don't have to check for other frameworks under the + // same role. We only break out of the innermost loop, so the next step + // will use the same slaveId, but a different role. + // The difference to the second `allocatable` check is that here we also + // check for revocable resources, which can be disabled on a per frame- + // work basis, which requires us to go through all frameworks in case we + // have allocatable revocable resources. + if (!allocatable(resources)) { + break; + } + // Remove revocable resources if the framework has not opted // for them. if (!frameworks[frameworkId].revocable) { @@ -1416,6 +1434,9 @@ void HierarchicalAllocatorProcess::allocate( } // If the resources are not allocatable, ignore. + // We can not break here, because another framework under the same role + // could accept revocable resources and breaking would skip all other + // frameworks. if (!allocatable(resources)) { continue; }
