subject:"Review Request 69821\: Enforced minimal allocatable resources in the hierarchical allocator."

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-13 Thread Benjamin Mahler


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212804
---


Ship it!




Ship It!

- Benjamin Mahler


On Feb. 13, 2019, 3:44 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 13, 2019, 3:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> 862dbb90bdfa39ead4b185104a308eabe249d734 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/12/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-13 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Feb. 13, 2019, 4:44 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Fix rebased errors


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its global options, it can now also inspects
framework-specific resource requirements. With that frameworks can e.g.,
configure resource requirements above the default minimal allocatable
resource, or opt into receiving resources considered too small to be
allocatable by the allocator in its default behavior.

For that we change the hierarchical allocator's `allocatable` function
to be framework and role-specific. As that does in some places not allow
us to abort iterating over candidate resource consumers like before an
additional check on whether any resources are left in an allocation
cycle is added as a break condition.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1420c2638786d85f7b04379e5d79e59990c3e6cf 
  src/master/allocator/mesos/hierarchical.cpp 
862dbb90bdfa39ead4b185104a308eabe249d734 


Diff: https://reviews.apache.org/r/69821/diff/12/

Changes: https://reviews.apache.org/r/69821/diff/11-12/


Testing
---

`make check`


File Attachments


Ratio new/old timings
  
https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-12 Thread Benjamin Mahler


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212759
---




src/master/allocator/mesos/hierarchical.cpp
Lines 2072-2074 (original)


Hm.. the first loop looks like a clean change, but this one removes the 
break condition?

(I guess this is because of the last suggestions + rebase?)



src/master/allocator/mesos/hierarchical.cpp
Lines 2085-2087 (patched)


Hm.. why don't we remove this and just let the continue happen below?



src/master/allocator/mesos/hierarchical.cpp
Line 2113 (original), 2113-2116 (patched)


Hm.. this seems to be moving the break from above down to here. Can we move 
it back up so that this diff is very minimal (i.e. like the changes in the 
quota loop above)


- Benjamin Mahler


On Feb. 11, 2019, 3:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 11, 2019, 3:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/11/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-11 Thread Benjamin Bannier



> On Feb. 8, 2019, 10:05 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1886-1888 (patched)
> > 
> >
> > Ditto the comment left below for the equivalent check in the second loop

Removed for now.


> On Feb. 8, 2019, 10:05 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2110-2112 (patched)
> > 
> >
> > Hm.. why bother with this? Seems correct without it and this doesn't 
> > seem to save much in terms of cost? It also seems like an orthogonal change?
> > 
> > It seems ok to add this one and the one above, but seems cleaner to do 
> > that in a follow up change that measures the performance impact? The quota 
> > one does seem to potentially save some expensive work

Removed for now.


> On Feb. 8, 2019, 10:05 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 2125 (original), 2138-2141 (patched)
> > 
> >
> > Let's not bother with this additional check? If `toAllocate` is empty, 
> > we'll continue from the check below (allocatable always is false for no 
> > resources), followed by the top of this loop breaking because there's 
> > nothing left for the role. So this only saves having to execute a few lines 
> > at the top of the loop? Doesn't seem worth it?

Removed.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212679
---


On Feb. 11, 2019, 4:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 11, 2019, 4:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/11/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-11 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Feb. 11, 2019, 4:46 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Rebase; address comments from Ben


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its global options, it can now also inspects
framework-specific resource requirements. With that frameworks can e.g.,
configure resource requirements above the default minimal allocatable
resource, or opt into receiving resources considered too small to be
allocatable by the allocator in its default behavior.

For that we change the hierarchical allocator's `allocatable` function
to be framework and role-specific. As that does in some places not allow
us to abort iterating over candidate resource consumers like before an
additional check on whether any resources are left in an allocation
cycle is added as a break condition.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1420c2638786d85f7b04379e5d79e59990c3e6cf 
  src/master/allocator/mesos/hierarchical.cpp 
bb9a9c95979f36c0564af5b3babb1c43077a363b 


Diff: https://reviews.apache.org/r/69821/diff/10/

Changes: https://reviews.apache.org/r/69821/diff/9-10/


Testing
---

`make check`


File Attachments


Ratio new/old timings
  
https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-08 Thread Benjamin Mahler


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212679
---



Looks good, just some minor comments. I suppose you'll need to rebase this off 
my updated patch (which I hope to get to today).


src/master/allocator/mesos/hierarchical.cpp
Lines 1886-1888 (patched)


Ditto the comment left below for the equivalent check in the second loop



src/master/allocator/mesos/hierarchical.cpp
Lines 2110-2112 (patched)


Hm.. why bother with this? Seems correct without it and this doesn't seem 
to save much in terms of cost? It also seems like an orthogonal change?

It seems ok to add this one and the one above, but seems cleaner to do that 
in a follow up change that measures the performance impact? The quota one does 
seem to potentially save some expensive work



src/master/allocator/mesos/hierarchical.cpp
Line 2125 (original), 2138-2141 (patched)


Let's not bother with this additional check? If `toAllocate` is empty, 
we'll continue from the check below (allocatable always is false for no 
resources), followed by the top of this loop breaking because there's nothing 
left for the role. So this only saves having to execute a few lines at the top 
of the loop? Doesn't seem worth it?


- Benjamin Mahler


On Feb. 8, 2019, 11:32 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 8, 2019, 11:32 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/9/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-08 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Feb. 8, 2019, 12:32 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Rebase onto https://reviews.apache.org/r/69902/


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its global options, it can now also inspects
framework-specific resource requirements. With that frameworks can e.g.,
configure resource requirements above the default minimal allocatable
resource, or opt into receiving resources considered too small to be
allocatable by the allocator in its default behavior.

For that we change the hierarchical allocator's `allocatable` function
to be framework and role-specific. As that does in some places not allow
us to abort iterating over candidate resource consumers like before an
additional check on whether any resources are left in an allocation
cycle is added as a break condition.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1420c2638786d85f7b04379e5d79e59990c3e6cf 
  src/master/allocator/mesos/hierarchical.cpp 
bb9a9c95979f36c0564af5b3babb1c43077a363b 


Diff: https://reviews.apache.org/r/69821/diff/9/

Changes: https://reviews.apache.org/r/69821/diff/8-9/


Testing
---

`make check`


File Attachments


Ratio new/old timings
  
https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-08 Thread Benjamin Bannier



> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2058-2060 (original), 2092-2094 (patched)
> > 
> >
> > We lost the comment here about why it's safe to break? It still seems 
> > relevant
> 
> Benjamin Bannier wrote:
> Hmm, not sure what you are after. My patch `continue`s instead of 
> `break`s. The way the code is structured now we cannot `break` here, but 
> instead must iterate over all frameworks. We could `break` if we'd e.g., made 
> `allocatable` independent of framework settings like before (e.g., by 
> computing a minimal allocatable resources given all framework information), 
> but we'd likely reject many allocation decision last minute that way in the 
> same spot where we currently check filters. The code as proposed here looks 
> simpler to me.
> 
> Can we drop this?

Dropping.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212523
---


On Feb. 8, 2019, 12:32 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 8, 2019, 12:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/9/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-05 Thread Benjamin Mahler


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212573
---



Much appreciated for having split out the changes! This is nice and small now :)

I had a hard time figuring out what the logic should be if MESOS-9554 isn't 
addressed by this patch, so I ended up pulling out a fix:

https://reviews.apache.org/r/69902/ (note the 1 small dependency patch in front 
of it)

We can base this patch on this fixed version of the code and it should be 
pretty clear. If we can land the fix quickly then we could just commit it and 
rebase this patch.


src/master/allocator/mesos/hierarchical.cpp
Lines 1985-1996 (original), 1985-1988 (patched)


It's hard to review this logic without fixing MESOS-9554, so I've put up a 
patch:

https://reviews.apache.org/r/69902/

Note that there's a small dependency pulled out in front of it to make it a 
smaller change. We can make this review depend on it, or probably easier, land 
that patch and then rebase this against master.


- Benjamin Mahler


On Feb. 5, 2019, 4:38 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 5, 2019, 4:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/8/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-05 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Feb. 5, 2019, 5:38 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Adress a couple issues raised by Ben


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its global options, it can now also inspects
framework-specific resource requirements. With that frameworks can e.g.,
configure resource requirements above the default minimal allocatable
resource, or opt into receiving resources considered too small to be
allocatable by the allocator in its default behavior.

For that we change the hierarchical allocator's `allocatable` function
to be framework and role-specific. As that does in some places not allow
us to abort iterating over candidate resource consumers like before an
additional check on whether any resources are left in an allocation
cycle is added as a break condition.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1420c2638786d85f7b04379e5d79e59990c3e6cf 
  src/master/allocator/mesos/hierarchical.cpp 
bb9a9c95979f36c0564af5b3babb1c43077a363b 


Diff: https://reviews.apache.org/r/69821/diff/8/

Changes: https://reviews.apache.org/r/69821/diff/7-8/


Testing
---

`make check`


File Attachments


Ratio new/old timings
  
https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-05 Thread Benjamin Bannier



> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2041-2042 (original), 2075-2076 (patched)
> > 
> >
> > Hm.. isn't the framework capability stripping messing with our break 
> > condition?

Hmm, it seems like it, yes. This should be an issue already now where we 
`break` in even more scenarios. This should be fixed outside of this change. I 
created https://issues.apache.org/jira/browse/MESOS-9554.

Dropping here.


> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2058-2060 (original), 2092-2094 (patched)
> > 
> >
> > We lost the comment here about why it's safe to break? It still seems 
> > relevant

Hmm, not sure what you are after. My patch `continue`s instead of `break`s. The 
way the code is structured now we cannot `break` here, but instead must iterate 
over all frameworks. We could `break` if we'd e.g., made `allocatable` 
independent of framework settings like before (e.g., by computing a minimal 
allocatable resources given all framework information), but we'd likely reject 
many allocation decision last minute that way in the same spot where we 
currently check filters. The code as proposed here looks simpler to me.

Can we drop this?


> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2064-2070 (original), 2096-2102 (patched)
> > 
> >
> > I'm left confused by the two checks now that they both continue, and I 
> > think the comment is now inaccurate and confusing? It is written based on 
> > break vs continue

I went over all the comments around resource emptiness and `allocatable` checks.


> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2090-2091 (original), 2122-2125 (patched)
> > 
> >
> > It seems more readable if this is a member function of `Framework`

In that case we'd have to pass in the allocator's `minAllocatableResources` 
(either here, or if they could change here). I'd suggest to keep this as is.

Dropping for now, feel free to reopen if you think this requires more 
discussion.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212523
---


On Feb. 5, 2019, 5:38 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 5, 2019, 5:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/8/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-04 Thread Mesos Reviewbot Windows


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212536
---



PASS: Mesos patch 69821 was successfully built and tested.

Reviews applied: `['69818', '69862', '69889', '69821']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2847/mesos-review-69821

- Mesos Reviewbot Windows


On Feb. 4, 2019, 10:05 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 4, 2019, 10:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-04 Thread Benjamin Bannier



> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote:
> > Can you split this change apart so that I can review more easily and we can 
> > land it faster?
> > 
> > (1) The plumbing and storage of the information: this is an easy change 
> > that doesn't require much thought, it looks good and can be shipped.
> > 
> > (2) The update to the allocation / allocatable logic: this is a harder 
> > change that warrants careful thought about the existing allocation code and 
> > I'd like to review it in isolation from the other changes in the current 
> > patch. Left some comments in the second stage that also apply to the first 
> > stage.
> > 
> > (3) Tests: also a rather easy change and would be good to ship in 
> > isolation. Haven't review this yet (would prefer to review in isolation 
> > since the review load is too high in this patch).

Done, created https://reviews.apache.org/r/69889/ and 
https://reviews.apache.org/r/69890/ which bracket this patch.

I fixed small issues in https://reviews.apache.org/r/69889/, which I didn't see 
in the bigger patch, so I have to agree with you that the patches where hard to 
review :/

Since you left a couple of allocator-related issues here already, I am reusing 
this RR for the allocator changes.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212523
---


On Feb. 4, 2019, 11:05 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 4, 2019, 11:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-04 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Feb. 4, 2019, 11:05 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Break out storage and test into https://reviews.apache.org/r/69889/ and 
https://reviews.apache.org/r/69890/, respectively


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its global options, it can now also inspects
framework-specific resource requirements. With that frameworks can e.g.,
configure resource requirements above the default minimal allocatable
resource, or opt into receiving resources considered too small to be
allocatable by the allocator in its default behavior.

For that we change the hierarchical allocator's `allocatable` function
to be framework and role-specific. As that does in some places not allow
us to abort iterating over candidate resource consumers like before an
additional check on whether any resources are left in an allocation
cycle is added as a break condition.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1420c2638786d85f7b04379e5d79e59990c3e6cf 
  src/master/allocator/mesos/hierarchical.cpp 
bb9a9c95979f36c0564af5b3babb1c43077a363b 


Diff: https://reviews.apache.org/r/69821/diff/7/

Changes: https://reviews.apache.org/r/69821/diff/6-7/


Testing
---

`make check`


File Attachments


Ratio new/old timings
  
https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-04 Thread Benjamin Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212523
---

Can you split this change apart so that I can review more easily and we can 
land it faster?

(1) The plumbing and storage of the information: this is an easy change that 
doesn't require much thought, it looks good and can be shipped.

(2) The update to the allocation / allocatable logic: this is a harder change 
that warrants careful thought about the existing allocation code and I'd like 
to review it in isolation from the other changes in the current patch. Left 
some comments in the second stage that also apply to the first stage.

(3) Tests: also a rather easy change and would be good to ship in isolation. 
Haven't review this yet (would prefer to review in isolation since the review 
load is too high in this patch).

src/master/allocator/mesos/hierarchical.cpp
Lines 2041-2042 (original), 2075-2076 (patched)

Hm.. isn't the framework capability stripping messing with our break 
condition?

src/master/allocator/mesos/hierarchical.cpp
Lines 2058-2060 (original), 2092-2094 (patched)

We lost the comment here about why it's safe to break? It still seems 
relevant

src/master/allocator/mesos/hierarchical.cpp
Lines 2064-2070 (original), 2096-2102 (patched)

I'm left confused by the two checks now that they both continue, and I 
think the comment is now inaccurate and confusing? It is written based on break 
vs continue

src/master/allocator/mesos/hierarchical.cpp
Lines 2090-2091 (original), 2122-2125 (patched)

It seems more readable if this is a member function of `Framework`

src/master/allocator/mesos/hierarchical.cpp
Lines 2455 (patched)

We already have the `Framework` in hand whenever we call this (and that's 
what I would expect), so can we simplify this? If we pass the `Framework` we 
can avoid having the contains guard and framework lookup logic.

src/master/framework.cpp
Lines 504 (patched)

Ah, here it is, should be in the earlier review?

- Benjamin Mahler

On Feb. 4, 2019, 8:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 4, 2019, 8:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-04 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Feb. 4, 2019, 9:37 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Move a comment to https://reviews.apache.org/r/69818/


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its global options, it can now also inspects
framework-specific resource requirements. With that frameworks can e.g.,
configure resource requirements above the default minimal allocatable
resource, or opt into receiving resources considered too small to be
allocatable by the allocator in its default behavior.

For that we change the hierarchical allocator's `allocatable` function
to be framework and role-specific. As that does in some places not allow
us to abort iterating over candidate resource consumers like before an
additional check on whether any resources are left in an allocation
cycle is added as a break condition.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1420c2638786d85f7b04379e5d79e59990c3e6cf 
  src/master/allocator/mesos/hierarchical.cpp 
bb9a9c95979f36c0564af5b3babb1c43077a363b 
  src/tests/hierarchical_allocator_tests.cpp 
cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 


Diff: https://reviews.apache.org/r/69821/diff/6/

Changes: https://reviews.apache.org/r/69821/diff/5-6/


Testing
---

`make check`


File Attachments


Ratio new/old timings
  
https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-04 Thread Mesos Reviewbot Windows


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212516
---



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69818', '69862', '69821']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2844/mesos-review-69821

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2844/mesos-review-69821/logs/mesos-tests.log):

```
W0204 12:23:02.096771 32304 slave.cpp:3934] Ignoring shutdown framework 
549ca5ea-b22e-492b-a25d-34e375705140- because it is terminating
I0204 12:23:02.098772 29556 master.cpp:1269] Agent 
549ca5ea-b22e-492b-a25d-34e375705140-S0 at slave(477)@192.10.1.6:64813 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0204 12:23:02.098772 29556 master.cpp:3272] Disconnecting agent 
549ca5ea-b22e-492b-a25d-34e375705140-S0 at slave(477)@192.10.1.6:64813 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0204 12:23:02.099773 29556 master.cpp:3291] Deactivating agent 
549ca5ea-b22e-492b-a25d-34e375705140-S0 at slave(477)@192.10.1.6:64813 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0204 12:23:02.099773 23200 hierarchical.cpp:395] Removed framework 
549ca5ea-b22e-492b-a25d-34e375705140-
I0204 12:23:02.099773 23200 hierarchical.cpp:830] Agent 
549ca5ea-b22e-492b-a25d-34e375705140-S0 deactivated
I0204 12:23:02.100780 23200 containerizer.cpp:2477] Destroying container 
e4bb3083-bbec-4218-875e-29d9d952976e in RUNNING state
I0204 12:23:02.100780 23200 containerizer.cpp:3144] Transitioning the state of 
container e4bb3083-bbec-4218-875e-29d9d952976e from RUNNING to DESTROYING
I0204 12:23:02.101773 23200 launcher.cpp:161] Asked to destroy container 
e4bb3083-bbec-4218-875e-29d9d952976e
W0204 12:23:02.102820 32144 process.cpp:838] Failed to recv on socket 
WindowsFD::Type::SOCKET=12116 to peer '192.10.1.6:50327': IO failed with error 
code: The specified network name is no longer available.

W0204 12:23:02.102820 32144 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=11820 to peer '192.10.1.6:50326': IO failed with error 
code: The specified network name is no longer available.

I0204 12:23:02.188532 29556 containerizer.cpp:2983] Container 
e4bb3083-bbec-4218-875e-29d9d952976e has exited
I0204 12:23:02.217483 26532 master.cpp:1109] Ma[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (682 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (698 ms total)

[--] Global test environment tear-down
[==] 1099 tests from 104 test cases ran. (500835 ms total)
[  PASSED  ] 1098 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

ster terminating
I0204 12:23:02.219491 20272 hierarchical.cpp:681] Removed agent 
549ca5ea-b22e-492b-a25d-34e375705140-S0
I0204 12:23:02.628484 32144 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Feb. 4, 2019, 11:14 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Feb. 4, 2019, 11:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
>   src/master/framework.cpp 4089cf4dfc65ac4dc5a092c04f4c2022ed8a1587 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-02-04 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Feb. 4, 2019, 12:14 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Add a clarifying comment


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its global options, it can now also inspects
framework-specific resource requirements. With that frameworks can e.g.,
configure resource requirements above the default minimal allocatable
resource, or opt into receiving resources considered too small to be
allocatable by the allocator in its default behavior.

For that we change the hierarchical allocator's `allocatable` function
to be framework and role-specific. As that does in some places not allow
us to abort iterating over candidate resource consumers like before an
additional check on whether any resources are left in an allocation
cycle is added as a break condition.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1420c2638786d85f7b04379e5d79e59990c3e6cf 
  src/master/allocator/mesos/hierarchical.cpp 
bb9a9c95979f36c0564af5b3babb1c43077a363b 
  src/master/framework.cpp 4089cf4dfc65ac4dc5a092c04f4c2022ed8a1587 
  src/tests/hierarchical_allocator_tests.cpp 
cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 


Diff: https://reviews.apache.org/r/69821/diff/5/

Changes: https://reviews.apache.org/r/69821/diff/4-5/


Testing
---

`make check`


File Attachments


Ratio new/old timings
  
https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-31 Thread Benjamin Mahler



> On Jan. 29, 2019, 8:37 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 2336 (patched)
> > 
> >
> > FrameworkEmptyMinAllocatable
> 
> Benjamin Bannier wrote:
> 

This and the one above are naming suggestions


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212421
---


On Jan. 31, 2019, 3:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 31, 2019, 3:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-31 Thread Mesos Reviewbot Windows


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212484
---



PASS: Mesos patch 69821 was successfully built and tested.

Reviews applied: `['69818', '69862', '69821']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2840/mesos-review-69821

- Mesos Reviewbot Windows


On Jan. 31, 2019, 3:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 31, 2019, 3:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its global options, it can now also inspects
> framework-specific resource requirements. With that frameworks can e.g.,
> configure resource requirements above the default minimal allocatable
> resource, or opt into receiving resources considered too small to be
> allocatable by the allocator in its default behavior.
> 
> For that we change the hierarchical allocator's `allocatable` function
> to be framework and role-specific. As that does in some places not allow
> us to abort iterating over candidate resource consumers like before an
> additional check on whether any resources are left in an allocation
> cycle is added as a break condition.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1420c2638786d85f7b04379e5d79e59990c3e6cf 
>   src/master/allocator/mesos/hierarchical.cpp 
> bb9a9c95979f36c0564af5b3babb1c43077a363b 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> File Attachments
> 
> 
> Ratio new/old timings
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-31 Thread Benjamin Bannier



> On Jan. 29, 2019, 9:37 p.m., Benjamin Mahler wrote:
> > The commit description makes it sound like the filter is using the 
> > **union** of the flag and the per-framework/role override:
> > 
> > > This set is populated with any minimal allocatable resources specified in 
> > > the allocator's options and any framework's minimal allocatable resources.
> > 
> > However, it should be an override (which is what the patch correctly does 
> > AFAICT). Probably, just using "override" in the explanation would make this 
> > very clear to the reader.
> > 
> > IMO, we only really need the first paragraph of the description:
> > 
> > > This patch modifies the hierarchical allocator to take 
> > > framework-specified minimal allocatable resources into account.
> > 
> > We could make it a bit clearer:
> > 
> > > This patch modifies the allocator to take the framework-specified minimum 
> > > allocatable resources into account. These act as an override of the 
> > > existing `min_allocatable_resources` flag.
> > 
> > What more needs to be said?

Sorry for not updating the commit message when posting my WIP result; updated 
now.


> On Jan. 29, 2019, 9:37 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1952-1968 (original), 1978-1987 (patched)
> > 
> >
> > This makes it look like we should consolidate into the `isFiltered` 
> > function?

See below.


> On Jan. 29, 2019, 9:37 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 2069 (original), 2082 (patched)
> > 
> >
> > Now that this is a continue instead of a break, it seems they can be 
> > consolidated into 1 below?
> > 
> > Also, can you post the benchmark numbers before and after this patch to 
> > make sure we're not accidentally regressing pretty badly on performance?

Let's keep this separated from the filtering step, if only to be able to 
document it more explicitly.

After our offline discussion I added an additional check just before this one 
which triggers a `continue` if `toAllocate.empty()`. This does prevent some 
useless work.

As a benchmark I ran 
`SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/4` 
(1000 agents, 500 frameworks). I patched the test to only execute 
`num_frameworks` allocation rounds, and also printed timings in `ms` instead of 
pretty-printing. The logs from an optimized build are:

1) `9f6ccbd41a55846e54297ecb31fddbeee3be50c9`
```
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/4
Using 1000 agents and 500 frameworks
Added 500 frameworks in 15.6022ms
Added 1000 agents in 82.2129ms
round 0 allocate() took 43.379ms to make 1000 offers after filtering 1000 offers
round 1 allocate() took 44.7003ms to make 1000 offers after filtering 2000 
offers
round 2 allocate() took 40.1724ms to make 1000 offers after filtering 3000 
offers
round 3 allocate() took 56.499ms to make 1000 offers after filtering 4000 offers
round 4 allocate() took 42.4679ms to make 1000 offers after filtering 5000 
offers
round 5 allocate() took 43.2156ms to make 1000 offers after filtering 6000 
offers
round 6 allocate() took 45.9366ms to make 1000 offers after filtering 7000 
offers
round 7 allocate() took 40.4537ms to make 1000 offers after filtering 8000 
offers
round 8 allocate() took 48.6968ms to make 1000 offers after filtering 9000 
offers
round 9 allocate() took 43.5264ms to make 1000 offers after filtering 1 
offers
round 10 allocate() took 42.1773ms to make 1000 offers after filtering 11000 
offers
round 11 allocate() took 37.3388ms to make 1000 offers after filtering 12000 
offers
round 12 allocate() took 40.3328ms to make 1000 offers after filtering 13000 
offers
round 13 allocate() took 43.9844ms to make 1000 offers after filtering 14000 
offers
round 14 allocate() took 36.8533ms to make 1000 offers after filtering 15000 
offers
round 15 allocate() took 41.1868ms to make 1000 offers after filtering 16000 
offers
round 16 allocate() took 44.1921ms to make 1000 offers after filtering 17000 
offers
round 17 allocate() took 44.6911ms to make 1000 offers after filtering 18000 
offers
round 18 allocate() took 40.2239ms to make 1000 offers after filtering 19000 
offers
round 19 allocate() took 46.5701ms to make 1000 offers after filtering 2 
offers
round 20 allocate() took 41.5735ms to make 1000 offers after filtering 21000 
offers
round 21 allocate() took 44.3456ms to make 1000 offers after filtering 22000 
offers
round 22 allocate() took 42.7894ms to make 1000 offers after filtering 23000 
offers
round 23 allocate() took 39.352ms to make 1000 offers after filtering 24000 
offers
round 24 allocate() took 40.5653ms to make 1000 offers after filtering 25000 
offers
round 25 allocate() took 46.8886ms to make 1000 offers after filtering 26000 
offers

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-31 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Jan. 31, 2019, 4:16 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Addressed comments from Ben here and offline.


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description (updated)
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its global options, it can now also inspects
framework-specific resource requirements. With that frameworks can e.g.,
configure resource requirements above the default minimal allocatable
resource, or opt into receiving resources considered too small to be
allocatable by the allocator in its default behavior.

For that we change the hierarchical allocator's `allocatable` function
to be framework and role-specific. As that does in some places not allow
us to abort iterating over candidate resource consumers like before an
additional check on whether any resources are left in an allocation
cycle is added as a break condition.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1420c2638786d85f7b04379e5d79e59990c3e6cf 
  src/master/allocator/mesos/hierarchical.cpp 
bb9a9c95979f36c0564af5b3babb1c43077a363b 
  src/tests/hierarchical_allocator_tests.cpp 
cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 


Diff: https://reviews.apache.org/r/69821/diff/4/

Changes: https://reviews.apache.org/r/69821/diff/3-4/


Testing
---

`make check`


File Attachments (updated)


Ratio new/old timings
  
https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-30 Thread Mesos Reviewbot Windows


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212456
---



PASS: Mesos patch 69821 was successfully built and tested.

Reviews applied: `['69861', '69818', '69862', '69821']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2833/mesos-review-69821

- Mesos Reviewbot Windows


On Jan. 30, 2019, 4:08 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 30, 2019, 4:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its options, it now inspects a dynamically
> calculated set of minimal allocatable resources. This set is populated
> with any minimal allocatable resources specified in the allocator's
> options and any framework's minimal allocatable resources. For a
> resource to be allocatable it needs to contain any of these minimal
> requirements.
> 
> If a framework does not specify minimal allocatable resource
> requirements, its minimal requirements are set to the globally
> configured option.
> 
> We also adjust the allocator to take a framework's minimal resource
> requirements into account when checking whether a resource can be
> allocated to a particular framework. The check is performed at the same
> time we check whether a framework filtered a particular resource. This
> avoids offering resources to frameworks which the framework would never
> have considered allocatable itself (e.g., given the global option of the
> allocator).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> ca1638390d89e2a81efd9d6d4a28b863c79723c4 
>   src/master/allocator/mesos/hierarchical.cpp 
> f1f3894058a8e3f008013cb269744bd36c0e31b3 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-30 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Jan. 30, 2019, 4:08 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

WIP.


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its options, it now inspects a dynamically
calculated set of minimal allocatable resources. This set is populated
with any minimal allocatable resources specified in the allocator's
options and any framework's minimal allocatable resources. For a
resource to be allocatable it needs to contain any of these minimal
requirements.

If a framework does not specify minimal allocatable resource
requirements, its minimal requirements are set to the globally
configured option.

We also adjust the allocator to take a framework's minimal resource
requirements into account when checking whether a resource can be
allocated to a particular framework. The check is performed at the same
time we check whether a framework filtered a particular resource. This
avoids offering resources to frameworks which the framework would never
have considered allocatable itself (e.g., given the global option of the
allocator).


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
ca1638390d89e2a81efd9d6d4a28b863c79723c4 
  src/master/allocator/mesos/hierarchical.cpp 
f1f3894058a8e3f008013cb269744bd36c0e31b3 
  src/tests/hierarchical_allocator_tests.cpp 
cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 


Diff: https://reviews.apache.org/r/69821/diff/3/

Changes: https://reviews.apache.org/r/69821/diff/2-3/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-30 Thread Mesos Reviewbot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212450
---



Patch looks great!

Reviews applied: [69818, 69819, 69820, 69821]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Jan. 29, 2019, 4:20 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 29, 2019, 4:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its options, it now inspects a dynamically
> calculated set of minimal allocatable resources. This set is populated
> with any minimal allocatable resources specified in the allocator's
> options and any framework's minimal allocatable resources. For a
> resource to be allocatable it needs to contain any of these minimal
> requirements.
> 
> If a framework does not specify minimal allocatable resource
> requirements, its minimal requirements are set to the globally
> configured option.
> 
> We also adjust the allocator to take a framework's minimal resource
> requirements into account when checking whether a resource can be
> allocated to a particular framework. The check is performed at the same
> time we check whether a framework filtered a particular resource. This
> avoids offering resources to frameworks which the framework would never
> have considered allocatable itself (e.g., given the global option of the
> allocator).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> ca1638390d89e2a81efd9d6d4a28b863c79723c4 
>   src/master/allocator/mesos/hierarchical.cpp 
> f1f3894058a8e3f008013cb269744bd36c0e31b3 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-29 Thread Benjamin Mahler


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212421
---



The commit description makes it sound like the filter is using the **union** of 
the flag and the per-framework/role override:

> This set is populated with any minimal allocatable resources specified in the 
> allocator's options and any framework's minimal allocatable resources.

However, it should be an override (which is what the patch correctly does 
AFAICT). Probably, just using "override" in the explanation would make this 
very clear to the reader.

IMO, we only really need the first paragraph of the description:

> This patch modifies the hierarchical allocator to take framework-specified 
> minimal allocatable resources into account.

We could make it a bit clearer:

> This patch modifies the allocator to take the framework-specified minimum 
> allocatable resources into account. These act as an override of the existing 
> `min_allocatable_resources` flag.

What more needs to be said?


src/master/allocator/mesos/hierarchical.cpp
Lines 1952-1968 (original), 1978-1987 (patched)


This makes it look like we should consolidate into the `isFiltered` 
function?



src/master/allocator/mesos/hierarchical.cpp
Line 2069 (original), 2082 (patched)


Now that this is a continue instead of a break, it seems they can be 
consolidated into 1 below?

Also, can you post the benchmark numbers before and after this patch to 
make sure we're not accidentally regressing pretty badly on performance?



src/master/allocator/mesos/hierarchical.cpp
Lines 2400-2409 (patched)


Hm.. what's this? Are you suggesting that the allocatable check should be 
moved within isFiltered? (seems like a good idea to me)



src/tests/hierarchical_allocator_tests.cpp
Lines 2287-2289 (patched)


Can you pull the tests into a separate review to simplify reviewing?



src/tests/hierarchical_allocator_tests.cpp
Lines 2289 (patched)


FrameworkMinAllocatable



src/tests/hierarchical_allocator_tests.cpp
Lines 2336 (patched)


FrameworkEmptyMinAllocatable



src/tests/hierarchical_allocator_tests.cpp
Lines 2378 (patched)


Hm.. this doesn't quite look right to me, it seems if a framework is 
setting the `min_allocatable_resources` field, it should override, but that's 
not happening here?


- Benjamin Mahler


On Jan. 29, 2019, 4:20 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 29, 2019, 4:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its options, it now inspects a dynamically
> calculated set of minimal allocatable resources. This set is populated
> with any minimal allocatable resources specified in the allocator's
> options and any framework's minimal allocatable resources. For a
> resource to be allocatable it needs to contain any of these minimal
> requirements.
> 
> If a framework does not specify minimal allocatable resource
> requirements, its minimal requirements are set to the globally
> configured option.
> 
> We also adjust the allocator to take a framework's minimal resource
> requirements into account when checking whether a resource can be
> allocated to a particular framework. The check is performed at the same
> time we check whether a framework filtered a particular resource. This
> avoids offering resources to frameworks which the framework would never
> have considered allocatable itself (e.g., given the global option of the
> allocator).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> ca1638390d89e2a81efd9d6d4a28b863c79723c4 
>   src/master/allocator/mesos/hierarchical.cpp 
> f1f3894058a8e3f008013cb269744bd36c0e31b3 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-29 Thread Mesos Reviewbot Windows


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212417
---



PASS: Mesos patch 69821 was successfully built and tested.

Reviews applied: `['69818', '69819', '69820', '69821']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2826/mesos-review-69821

- Mesos Reviewbot Windows


On Jan. 29, 2019, 4:20 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 29, 2019, 4:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its options, it now inspects a dynamically
> calculated set of minimal allocatable resources. This set is populated
> with any minimal allocatable resources specified in the allocator's
> options and any framework's minimal allocatable resources. For a
> resource to be allocatable it needs to contain any of these minimal
> requirements.
> 
> If a framework does not specify minimal allocatable resource
> requirements, its minimal requirements are set to the globally
> configured option.
> 
> We also adjust the allocator to take a framework's minimal resource
> requirements into account when checking whether a resource can be
> allocated to a particular framework. The check is performed at the same
> time we check whether a framework filtered a particular resource. This
> avoids offering resources to frameworks which the framework would never
> have considered allocatable itself (e.g., given the global option of the
> allocator).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> ca1638390d89e2a81efd9d6d4a28b863c79723c4 
>   src/master/allocator/mesos/hierarchical.cpp 
> f1f3894058a8e3f008013cb269744bd36c0e31b3 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-29 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

(Updated Jan. 29, 2019, 5:20 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Rebased.


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its options, it now inspects a dynamically
calculated set of minimal allocatable resources. This set is populated
with any minimal allocatable resources specified in the allocator's
options and any framework's minimal allocatable resources. For a
resource to be allocatable it needs to contain any of these minimal
requirements.

If a framework does not specify minimal allocatable resource
requirements, its minimal requirements are set to the globally
configured option.

We also adjust the allocator to take a framework's minimal resource
requirements into account when checking whether a resource can be
allocated to a particular framework. The check is performed at the same
time we check whether a framework filtered a particular resource. This
avoids offering resources to frameworks which the framework would never
have considered allocatable itself (e.g., given the global option of the
allocator).


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
ca1638390d89e2a81efd9d6d4a28b863c79723c4 
  src/master/allocator/mesos/hierarchical.cpp 
f1f3894058a8e3f008013cb269744bd36c0e31b3 
  src/tests/hierarchical_allocator_tests.cpp 
cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 


Diff: https://reviews.apache.org/r/69821/diff/2/

Changes: https://reviews.apache.org/r/69821/diff/1-2/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-23 Thread Mesos Reviewbot Windows


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/#review212240
---



PASS: Mesos patch 69821 was successfully built and tested.

Reviews applied: `['69818', '69819', '69820', '69821']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2811/mesos-review-69821

- Mesos Reviewbot Windows


On Jan. 23, 2019, 12:12 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 23, 2019, 12:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its options, it now inspects a dynamically
> calculated set of minimal allocatable resources. This set is populated
> with any minimal allocatable resources specified in the allocator's
> options and any framework's minimal allocatable resources. For a
> resource to be allocatable it needs to contain any of these minimal
> requirements.
> 
> If a framework does not specify minimal allocatable resource
> requirements, its minimal requirements are set to the globally
> configured option.
> 
> We also adjust the allocator to take a framework's minimal resource
> requirements into account when checking whether a resource can be
> allocated to a particular framework. The check is performed at the same
> time we check whether a framework filtered a particular resource. This
> avoids offering resources to frameworks which the framework would never
> have considered allocatable itself (e.g., given the global option of the
> allocator).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> ca1638390d89e2a81efd9d6d4a28b863c79723c4 
>   src/master/allocator/mesos/hierarchical.cpp 
> f1f3894058a8e3f008013cb269744bd36c0e31b3 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-23 Thread Benjamin Bannier


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69821/
---

Review request for mesos, Benjamin Mahler and Meng Zhu.


Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its options, it now inspects a dynamically
calculated set of minimal allocatable resources. This set is populated
with any minimal allocatable resources specified in the allocator's
options and any framework's minimal allocatable resources. For a
resource to be allocatable it needs to contain any of these minimal
requirements.

If a framework does not specify minimal allocatable resource
requirements, its minimal requirements are set to the globally
configured option.

We also adjust the allocator to take a framework's minimal resource
requirements into account when checking whether a resource can be
allocated to a particular framework. The check is performed at the same
time we check whether a framework filtered a particular resource. This
avoids offering resources to frameworks which the framework would never
have considered allocatable itself (e.g., given the global option of the
allocator).


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
ca1638390d89e2a81efd9d6d4a28b863c79723c4 
  src/master/allocator/mesos/hierarchical.cpp 
f1f3894058a8e3f008013cb269744bd36c0e31b3 
  src/tests/hierarchical_allocator_tests.cpp 
cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 


Diff: https://reviews.apache.org/r/69821/diff/1/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

30 matches

Site Navigation

Mail list logo

Footer information