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 Bannier <be...@mesosphere.io> on 2019/02/04 11:14:37 UTC

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

-----------------------------------------------------------
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.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
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.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
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.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69821/#comment298666>

    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)
<https://reviews.apache.org/r/69821/#comment298667>

    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)
<https://reviews.apache.org/r/69821/#comment298668>

    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.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
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.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On Feb. 8, 2019, 10:05 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1886-1888 (patched)
> > <https://reviews.apache.org/r/69821/diff/9/?file=2124578#file2124578line1886>
> >
> >     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)
> > <https://reviews.apache.org/r/69821/diff/9/?file=2124578#file2124578line2110>
> >
> >     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)
> > <https://reviews.apache.org/r/69821/diff/9/?file=2124578#file2124578line2138>
> >
> >     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.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69821/#comment298529>

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



src/master/allocator/mesos/hierarchical.cpp
Lines 2110-2112 (patched)
<https://reviews.apache.org/r/69821/#comment298528>

    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)
<https://reviews.apache.org/r/69821/#comment298527>

    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.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
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.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69821/#comment298401>

    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.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
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.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
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.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2041-2042 (original), 2075-2076 (patched)
> > <https://reviews.apache.org/r/69821/diff/5/?file=2123824#file2123824line2084>
> >
> >     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)
> > <https://reviews.apache.org/r/69821/diff/5/?file=2123824#file2123824line2101>
> >
> >     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)
> > <https://reviews.apache.org/r/69821/diff/5/?file=2123824#file2123824line2107>
> >
> >     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)
> > <https://reviews.apache.org/r/69821/diff/5/?file=2123824#file2123824line2133>
> >
> >     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.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2058-2060 (original), 2092-2094 (patched)
> > <https://reviews.apache.org/r/69821/diff/5/?file=2123824#file2123824line2101>
> >
> >     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.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> 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.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69821/#comment298332>

    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)
<https://reviews.apache.org/r/69821/#comment298329>

    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)
<https://reviews.apache.org/r/69821/#comment298330>

    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)
<https://reviews.apache.org/r/69821/#comment298328>

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



src/master/allocator/mesos/hierarchical.cpp
Lines 2455 (patched)
<https://reviews.apache.org/r/69821/#comment298327>

    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)
<https://reviews.apache.org/r/69821/#comment298325>

    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.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
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.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
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-0000 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-0000
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 
>   src/tests/hierarchical_allocator_tests.cpp cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/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
> 
>