You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2017/01/25 21:55:12 UTC
Review Request 55870: Update the allocator to handle frameworks with
multiple roles.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
Bugs: MESOS-6635
https://issues.apache.org/jira/browse/MESOS-6635
Repository: mesos
Description
-------
The allocator now sets `Resource.AllocationInfo` when performing
an allocation, and the interface is updated to expose allocations
for each role within a framework.
The allocator also assumes that `Resource.AllocationInfo` is set
for inbound resources that are allocated (e.g. when adding an agent,
when adding a framework, when recovering resources). Note however,
that the necessary changes to the master and agent to enforce this
will be done via separate patches.
Diffs
-----
include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
Diff: https://reviews.apache.org/r/55870/diff/
Testing
-------
Adjustments to existing tests are split out into a subsequent patch.
New tests for frameworks having multiple roles will be added as a follow up.
Thanks,
Benjamin Mahler
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Jan. 28, 2017, 2:11 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 827-831
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line827>
> >
> > I found that this was used in many places for both master and agent, how about put this in resources_utils.cpp?
Will leave a TODO since I'm not sure what the best approach is yet, thanks!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163404
-----------------------------------------------------------
On Jan. 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163404
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp (lines 820 - 824)
<https://reviews.apache.org/r/55870/#comment234862>
I found that this was used in many places for both master and agent, how about put this in resources_utils.cpp?
- Guangya Liu
On \u4e00\u6708 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Jan. 31, 2017, 5:25 a.m., Michael Park wrote:
> >
Thanks!
> On Jan. 31, 2017, 5:25 a.m., Michael Park wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 667-676
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line667>
> >
> > We have a bunch of places where this condition needs to hold true I think. Is this location more special than others for some reason?
> >
> > If we're not going to pull this out to a validation of some kind, let's at least leave a `TODO` here.
I'll go with this:
```
hashmap<string, Resources> allocations = offeredResources.allocations();
CHECK_EQ(1u, allocations.size());
string role = allocations.begin()->first;
```
And put a TODO that it is inducing unnecessary copying of Resources.
> On Jan. 31, 2017, 5:25 a.m., Michael Park wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 436
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line436>
> >
> > Does `CHECK_EQ` not work here?
No unfortunately not, since we only have a `stringify()` definition for `std::set`. We don't have an `operator<<` defined.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163591
-----------------------------------------------------------
On Jan. 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163591
-----------------------------------------------------------
Fix it, then Ship it!
src/master/allocator/mesos/hierarchical.hpp (line 355)
<https://reviews.apache.org/r/55870/#comment235061>
`s/strippedAllocation/unallocated/`, since "stripped" means something different for us.
src/master/allocator/mesos/hierarchical.cpp (line 431)
<https://reviews.apache.org/r/55870/#comment235062>
Does `CHECK_EQ` not work here?
src/master/allocator/mesos/hierarchical.cpp (lines 661 - 670)
<https://reviews.apache.org/r/55870/#comment235063>
We have a bunch of places where this condition needs to hold true I think. Is this location more special than others for some reason?
If we're not going to pull this out to a validation of some kind, let's at least leave a `TODO` here.
- Michael Park
On Jan. 25, 2017, 1:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 1:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Jan. 30, 2017, 4:12 p.m., Benjamin Bannier wrote:
> >
Thanks!
> On Jan. 30, 2017, 4:12 p.m., Benjamin Bannier wrote:
> > include/mesos/allocator/allocator.hpp, lines 89-92
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615402#file1615402line89>
> >
> > What is the reason for changing this interface? We should think hard before making it too specific to requirements from single features such as `MULTI_ROLE`.
> >
> > Propagating internal assumptions such as "only allocations towards the same role in one `Resources`" across module boundaries really limits room for future developments. In this case both our allocator and the master rely on above assumption; it should be possible for them to adjust passed resources on interfaces, e.g., by creating datastructure like you proposed here internally themself (they could use the same helper function). That way we could avoid tech debt spilling into interfaces.
>
> Michael Park wrote:
> I can sympathize with this position, although it doesn't seem terrible to me that
> the notion of multi-role which is being introduced to Mesos (rather than a specific allocator)
> changes the allocator API.
>
> But it doesn't seem obvious to me why it should be `hashmap<string, hashmap<SlaveID, Resources>>`,
> as opposed to `hashmap<SlaveID, hashmap<string, Resources>>`. I tried to look through the code
> briefly to see if it happens to make the code a lot simpler or something, but I don't really see one..
>
> It seems to me like the pattern I see in a few places could be used here without changing the API also.
>
> ```cpp
> const hashmap<SlaveID, Resources>& resources;
>
> foreachpair (const SlaveID& slaveId, const Resources& resources) {
> foreachpair (const string& role,
> const Resources& resources,
> resources.allocation()) {
> // ...
> }
> }
> ```
>
> I'm more-so trying to understand what the reasoning is here.
A couple of thoughts here:
Allocations occur to roles. Frameworks that are subscribed to the role can receive the allocations. A historical artifact is that we have a second level of implicitly weighted fair sharing between frameworks in a role to maintain our "pessimistic" offer technique, and so the allocator has knowlege of the frameworks. This approach was unfortunate since we have the framework id acting as a pseudo role but there is no control over the weights or quota for this pseudo role. The vision is that we should just perform allocations to roles, what this would mean is that if there are multiple frameworks listening to a role, they compete in a first come first serve fashion (if this is not desired, sub-roles can be used to impose sharing and quota constraints between the frameworks).
Given this, I suspect we could design the allocator to have no knowledge of what a framework is, and rather have the allocator operate purely on roles. This would lead to a "role" centric API. In this model, the master understands how to deliver the allocations to the roles based on which frameworks are subscribed to the role. The only wrinkle here that I see is that framework capabilities have an influence on allocation.
This is why I made the roles explicit, to reflect that allocations are occuring to roles. Thoughts?
> On Jan. 30, 2017, 4:12 p.m., Benjamin Bannier wrote:
> > include/mesos/allocator/allocator.hpp, line 264
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615402#file1615402line264>
> >
> > Do we need this detail of our allocator implementation in the generic interface?
Hm.. I'm not sure I follow how this is an implementation detail, it's currently an invariant we impose on the caller, so the caller should know about it, no?
> On Jan. 30, 2017, 4:12 p.m., Benjamin Bannier wrote:
> > include/mesos/allocator/allocator.hpp, lines 333-335
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615402#file1615402line333>
> >
> > Internal detail?
Hm.. I'm not sure I follow how this is an implementation detail, it's currently an invariant we impose on the caller, so the caller should know about it, no?
> On Jan. 30, 2017, 4:12 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 691-696
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line691>
> >
> > Is this detailed knowledge of `Resource` internals necessary here? Below we use the same adjusted `operation` regardless of its type.
I'll remove the injection and leave the validation TODO.
> On Jan. 30, 2017, 4:12 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 709-710
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line709>
> >
> > Lets try to put this code into the caller (e.g., `Master::_accept`). It seems that would introduce less current and future breakage in custom allocator modules.
I'll remove the injection and leave the validation TODO.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163516
-----------------------------------------------------------
On Jan. 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Michael Park <mp...@apache.org>.
> On Jan. 30, 2017, 8:12 a.m., Benjamin Bannier wrote:
> > include/mesos/allocator/allocator.hpp, lines 89-92
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615402#file1615402line89>
> >
> > What is the reason for changing this interface? We should think hard before making it too specific to requirements from single features such as `MULTI_ROLE`.
> >
> > Propagating internal assumptions such as "only allocations towards the same role in one `Resources`" across module boundaries really limits room for future developments. In this case both our allocator and the master rely on above assumption; it should be possible for them to adjust passed resources on interfaces, e.g., by creating datastructure like you proposed here internally themself (they could use the same helper function). That way we could avoid tech debt spilling into interfaces.
I can sympathize with this position, although it doesn't seem terrible to me that
the notion of multi-role which is being introduced to Mesos (rather than a specific allocator)
changes the allocator API.
But it doesn't seem obvious to me why it should be `hashmap<string, hashmap<SlaveID, Resources>>`,
as opposed to `hashmap<SlaveID, hashmap<string, Resources>>`. I tried to look through the code
briefly to see if it happens to make the code a lot simpler or something, but I don't really see one..
It seems to me like the pattern I see in a few places could be used here without changing the API also.
```cpp
const hashmap<SlaveID, Resources>& resources;
foreachpair (const SlaveID& slaveId, const Resources& resources) {
foreachpair (const string& role,
const Resources& resources,
resources.allocation()) {
// ...
}
}
```
I'm more-so trying to understand what the reasoning is here.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163516
-----------------------------------------------------------
On Jan. 25, 2017, 1:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 1:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163516
-----------------------------------------------------------
include/mesos/allocator/allocator.hpp (lines 89 - 92)
<https://reviews.apache.org/r/55870/#comment234977>
What is the reason for changing this interface? We should think hard before making it too specific to requirements from single features such as `MULTI_ROLE`.
Propagating internal assumptions such as "only allocations towards the same role in one `Resources`" across module boundaries really limits room for future developments. In this case both our allocator and the master rely on above assumption; it should be possible for them to adjust passed resources on interfaces, e.g., by creating datastructure like you proposed here internally themself (they could use the same helper function). That way we could avoid tech debt spilling into interfaces.
include/mesos/allocator/allocator.hpp (line 264)
<https://reviews.apache.org/r/55870/#comment234979>
Do we need this detail of our allocator implementation in the generic interface?
include/mesos/allocator/allocator.hpp (lines 333 - 335)
<https://reviews.apache.org/r/55870/#comment234980>
Internal detail?
src/master/allocator/mesos/hierarchical.cpp (lines 685 - 690)
<https://reviews.apache.org/r/55870/#comment234978>
Is this detailed knowledge of `Resource` internals necessary here? Below we use the same adjusted `operation` regardless of its type.
src/master/allocator/mesos/hierarchical.cpp (lines 703 - 704)
<https://reviews.apache.org/r/55870/#comment234976>
Lets try to put this code into the caller (e.g., `Master::_accept`). It seems that would introduce less current and future breakage in custom allocator modules.
- Benjamin Bannier
On Jan. 25, 2017, 10:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 10:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Guangya Liu <gy...@gmail.com>.
> On \u4e00\u6708 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1133-1137
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line1133>
> >
> > Add `role` here in the log message?
>
> Benjamin Mahler wrote:
> Hm.. not sure what you mean, the role will be displayed when printing the resources. Can you clarify?
I see, the `role` is already in `resource`, so no need to add it.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163388
-----------------------------------------------------------
On \u4e00\u6708 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Guangya Liu <gy...@gmail.com>.
> On \u4e00\u6708 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1176-1177
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line1176>
> >
> > A question here: Why not call `resources.unallocate()` at #1130?
> >
> > This can make the logic of `allocate` and `recoverResources` as pair: `allocate` will set allocation info while `recoverResources` will clear allocatio info.
I see, it is enough to only decrese the `allocated` in `recoverResources`.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163388
-----------------------------------------------------------
On \u4e00\u6708 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Jan. 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > include/mesos/allocator/allocator.hpp, line 91
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615402#file1615402line91>
> >
> > I think that we also need to reflect this in CHANGELOG to clarify that this interface was updated for multi_role support.
Sounds good, I added to the description of [MESOS-6762](https://issues.apache.org/jira/browse/MESOS-6762).
> On Jan. 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 283
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615404#file1615404line283>
> >
> > I think that the reason you want to update here is want to keep consistent, right?
Yes
> On Jan. 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1133-1137
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line1133>
> >
> > Add `role` here in the log message?
Hm.. not sure what you mean, the role will be displayed when printing the resources. Can you clarify?
> On Jan. 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1133-1138
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line1133>
> >
> > Add `role` here in the log message?
Ditto here.
> On Jan. 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1231-1232
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line1231>
> >
> > Nit, how about update the message a bit:
> >
> > ```
> > Suppressed offers for roles {r1,r2} in framework xx-xx-xx-xx
> > ```
> >
> > Ditto for the log message in `reviveOffers`.
Sure, sounds good.
> On Jan. 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1455
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line1455>
> >
> > s/role/roles
This is referring to the reserved role, so this should remain singular.
> On Jan. 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1495
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line1495>
> >
> > Why highlight the `unallocate()` here, I think that this was already done via `Resources::createStrippedScalarQuantity` and the resources being `flatten` here should not have `allocation info`.
Yes, sorry! This became stale as I was writing the code.
> On Jan. 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 2059-2061
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line2059>
> >
> > Adding `role` in the log message here to clarify which role on this framework is being filtered?
Sounds good!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163388
-----------------------------------------------------------
On Jan. 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163388
-----------------------------------------------------------
include/mesos/allocator/allocator.hpp (line 91)
<https://reviews.apache.org/r/55870/#comment234820>
I think that we also need to reflect this in CHANGELOG to clarify that this interface was updated for multi_role support.
src/master/allocator/mesos/hierarchical.hpp (line 232)
<https://reviews.apache.org/r/55870/#comment234821>
How about update the comments a bit?
```
// Remove an offer filter for the specified role of a framework.
```
src/master/allocator/mesos/hierarchical.hpp (lines 251 - 252)
<https://reviews.apache.org/r/55870/#comment234822>
How about update the comments a bit?
```
// Returns true if there is a resource offer filter for the
// speficied role of the framework on this slave.
```
src/master/allocator/mesos/hierarchical.hpp (line 283)
<https://reviews.apache.org/r/55870/#comment234823>
I think that the reason you want to update here is want to keep consistent, right?
src/master/allocator/mesos/hierarchical.cpp (line 428)
<https://reviews.apache.org/r/55870/#comment234824>
s/a new 'role'/new 'roles'
src/master/allocator/mesos/hierarchical.cpp (lines 1124 - 1128)
<https://reviews.apache.org/r/55870/#comment234841>
Add `role` here in the log message?
src/master/allocator/mesos/hierarchical.cpp (lines 1124 - 1129)
<https://reviews.apache.org/r/55870/#comment234842>
Add `role` here in the log message?
src/master/allocator/mesos/hierarchical.cpp (lines 1167 - 1168)
<https://reviews.apache.org/r/55870/#comment234843>
A question here: Why not call `resources.unallocate()` at #1130?
This can make the logic of `allocate` and `recoverResources` as pair: `allocate` will set allocation info while `recoverResources` will clear allocatio info.
src/master/allocator/mesos/hierarchical.cpp (lines 1219 - 1220)
<https://reviews.apache.org/r/55870/#comment234836>
Nit, how about update the message a bit:
```
Suppressed offers for roles {r1,r2} in framework xx-xx-xx-xx
```
Ditto for the log message in `reviveOffers`.
src/master/allocator/mesos/hierarchical.cpp (line 1440)
<https://reviews.apache.org/r/55870/#comment234837>
s/role/roles
src/master/allocator/mesos/hierarchical.cpp (line 1480)
<https://reviews.apache.org/r/55870/#comment234838>
Why highlight the `unallocate()` here, I think that this was already done via `Resources::createStrippedScalarQuantity` and the resources being `flatten` here should not have `allocation info`.
src/master/allocator/mesos/hierarchical.cpp (lines 2044 - 2046)
<https://reviews.apache.org/r/55870/#comment234839>
Adding `role` in the log message here to clarify which role on this framework is being filtered?
src/master/allocator/mesos/hierarchical.cpp (lines 2044 - 2046)
<https://reviews.apache.org/r/55870/#comment234840>
Add `role` in the log message here to clarify which role on this framework is being filtered?
- Guangya Liu
On \u4e00\u6708 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Jan. 31, 2017, 6:18 a.m., Michael Park wrote:
> > src/master/master.cpp, lines 6541-6544
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615407#file1615407line6541>
> >
> > Can't we just use `foreachpair` for both?
> >
> > ```cpp
> > foreachpair (const string& role,
> > const hashmap<SlaveID, Resources>& resources_,
> > resources) {
> > foreachpair (const SlaveID& slaveId,
> > const Resources& offered,
> > resources_) {
> > // ...
> > }
> > }
> > ```
We could, just seemed a bit unfortunate to introduce a similarly named "resources" variable.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163625
-----------------------------------------------------------
On Jan. 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 9:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 55870: Update the allocator to handle frameworks
with multiple roles.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55870/#review163625
-----------------------------------------------------------
Fix it, then Ship it!
src/master/master.cpp (lines 6541 - 6544)
<https://reviews.apache.org/r/55870/#comment235098>
Can't we just use `foreachpair` for both?
```cpp
foreachpair (const string& role,
const hashmap<SlaveID, Resources>& resources_,
resources) {
foreachpair (const SlaveID& slaveId,
const Resources& offered,
resources_) {
// ...
}
}
```
- Michael Park
On Jan. 25, 2017, 1:55 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 1:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
>
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 558594983beb6ff49c1fddf875ba29c1983b1288
> src/master/allocator/mesos/allocator.hpp 8e0f37a5eedd4d71d765991c0039b49c96f0ca53
> src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b
> src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba
> src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558
>
> Diff: https://reviews.apache.org/r/55870/diff/
>
>
> Testing
> -------
>
> Adjustments to existing tests are split out into a subsequent patch.
>
> New tests for frameworks having multiple roles will be added as a follow up.
>
>
> Thanks,
>
> Benjamin Mahler
>
>