You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2018/06/20 05:34:00 UTC

Review Request 67669: Removed an invariant check when updating the hierarchical allocator.

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

Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.


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


Repository: mesos


Description
-------

`HierarchicalAllocatorProcess::updateAllocation` checks that the given
resource conversions should not change the allocation quantities of the
specified framework. Although this is true for speculative operations,
this might not hold for arbitrary resource conversions in general.

Since the allocator does not rely on this invariant, this patch relaxes
the invariant to make resource conversions more future-proof.


Diffs
-----

  src/master/allocator/mesos/hierarchical.cpp b558228290e5ae6dbcc1b8a6e1fe69db9fd5874c 


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


Testing
-------

sudo make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On June 20, 2018, 2:20 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 921 (original)
> > <https://reviews.apache.org/r/67669/diff/1/?file=2042688#file2042688line924>
> >
> >     While this might not be required by the allocator, generally an operation changing the resource quantity is not supported by the `Sorter` interface.
> >     
> >     We already rely on the invariant being checked here in above calls to `Sorter::update` which explicit disallow such updates. In order to support operations changing the quantity of the stripped resources we need to do additional work.
> 
> Chun-Hung Hsiao wrote:
>     I added a unit test and updated the description of `Sorter::update`, but I'm hesitating to add some more concrete checks. We discussed about checking the resources names don't change, but changing names are essentially removing the old name and adding a new one, so it seems not consistent to have such checks.
>     
>     Another concern I have is that we now rely on the caller to update the allocation and the total resources synchronously, but I cannot find a good way to validate that. Suggestions?

After discussed with BenM, I'm keeping the check, but just relax it to support what we need.


- Chun-Hung


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


On July 6, 2018, 11:27 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated July 6, 2018, 11:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-8995
>     https://issues.apache.org/jira/browse/MESOS-8995
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch allows the allocator to remove resources when calling
> `Sorter::update`, so Mesos can remove resources that are no longer
> available through resource conversions.
> 
> An example usage is that we can make `DESTROY_VOLUME` to convert a
> volume with a stale profile to an empty resource, so the disk space
> will disappear instead of being offered out to other frameworks.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG d0cc0e0a4d2a7aa69cadc21923c7af397ebcbb25 
>   docs/upgrades.md cc8a251945af27a432dbeb5585b998cd84e35b22 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
>   src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
>   src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
>   src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/3/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Removed an invariant check when updating the hierarchical allocator.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On June 20, 2018, 2:20 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 921 (original)
> > <https://reviews.apache.org/r/67669/diff/1/?file=2042688#file2042688line924>
> >
> >     While this might not be required by the allocator, generally an operation changing the resource quantity is not supported by the `Sorter` interface.
> >     
> >     We already rely on the invariant being checked here in above calls to `Sorter::update` which explicit disallow such updates. In order to support operations changing the quantity of the stripped resources we need to do additional work.

I added a unit test and updated the description of `Sorter::update`, but I'm hesitating to add some more concrete checks. We discussed about checking the resources names don't change, but changing names are essentially removing the old name and adding a new one, so it seems not consistent to have such checks.

Another concern I have is that we now rely on the caller to update the allocation and the total resources synchronously, but I cannot find a good way to validate that. Suggestions?


- Chun-Hung


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


On June 20, 2018, 5:34 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated June 20, 2018, 5:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-8995
>     https://issues.apache.org/jira/browse/MESOS-8995
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `HierarchicalAllocatorProcess::updateAllocation` checks that the given
> resource conversions should not change the allocation quantities of the
> specified framework. Although this is true for speculative operations,
> this might not hold for arbitrary resource conversions in general.
> 
> Since the allocator does not rely on this invariant, this patch relaxes
> the invariant to make resource conversions more future-proof.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp b558228290e5ae6dbcc1b8a6e1fe69db9fd5874c 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/1/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Removed an invariant check when updating 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/67669/#review205079
-----------------------------------------------------------




src/master/allocator/mesos/hierarchical.cpp
Line 921 (original)
<https://reviews.apache.org/r/67669/#comment288036>

    While this might not be required by the allocator, generally an operation changing the resource quantity is not supported by the `Sorter` interface.
    
    We already rely on the invariant being checked here in above calls to `Sorter::update` which explicit disallow such updates. In order to support operations changing the quantity of the stripped resources we need to do additional work.


- Benjamin Bannier


On June 20, 2018, 7:34 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated June 20, 2018, 7:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-8995
>     https://issues.apache.org/jira/browse/MESOS-8995
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `HierarchicalAllocatorProcess::updateAllocation` checks that the given
> resource conversions should not change the allocation quantities of the
> specified framework. Although this is true for speculative operations,
> this might not hold for arbitrary resource conversions in general.
> 
> Since the allocator does not rely on this invariant, this patch relaxes
> the invariant to make resource conversions more future-proof.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp b558228290e5ae6dbcc1b8a6e1fe69db9fd5874c 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/1/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On July 10, 2018, 3:33 p.m., Benjamin Bannier wrote:
> > docs/upgrades.md
> > Lines 469 (patched)
> > <https://reviews.apache.org/r/67669/diff/4/?file=2056894#file2056894line469>
> >
> >     I don't think we do need to mention any operations. What about e.g.,
> >     
> >     > The semantics of `Sorter::update` has been changed so that resources can be removed certain agents in the pool without removing the full agent. Users are expected to update the total resources as well by e.g., removing the old and adding back the new resources for an agent.

Updated the text with some adjustments.


> On July 10, 2018, 3:33 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 887 (patched)
> > <https://reviews.apache.org/r/67669/diff/4/?file=2056895#file2056895line887>
> >
> >     Why are changes to the allocator in this file? Should they go somewhere else where we enable a feature or explicitly fix a bug? We should probably also add a test.
> 
> Chun-Hung Hsiao wrote:
>     We don't change the behavior of the allocator. However, the original CHECK prohibited any resource removal, so we need to change the CHECK. The newly introduced variable is just for rewriting the new CHECK.
>     
>     Since this is a CHECK, it would not be easy to test the violation. But I'll add an allocator test to make sure that the CHECK passes when we reduce resources.

Moved the allocator changes to r/67876.


> On July 10, 2018, 3:33 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 924-926 (original), 934-939 (patched)
> > <https://reviews.apache.org/r/67669/diff/4/?file=2056895#file2056895line936>
> >
> >     I believe we do not need the potentially expensive calls to `toUnreserved` here as `createStrippedScalarQuantity` creates unreserved resources.
> 
> Chun-Hung Hsiao wrote:
>     You're right. The `createStrippedScalarQuantity()` implementation is changed recently. Previously we keep the static reservation and thus the additional `toUnreserved()` call.

Moved the allocator changes to r/67876.


- Chun-Hung


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


On July 10, 2018, 5:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated July 10, 2018, 5:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-9015
>     https://issues.apache.org/jira/browse/MESOS-9015
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch allows the allocator to remove resources when calling
> `Sorter::update`, so Mesos can remove resources that are no longer
> available through resource conversions.
> 
> An example usage is that we can make `DESTROY_VOLUME` to convert a
> volume with a stale profile to an empty resource, so the disk space
> will disappear instead of being offered out to other frameworks.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
>   docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
>   src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
>   src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
>   src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/4/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On July 10, 2018, 3:33 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 887 (patched)
> > <https://reviews.apache.org/r/67669/diff/4/?file=2056895#file2056895line887>
> >
> >     Why are changes to the allocator in this file? Should they go somewhere else where we enable a feature or explicitly fix a bug? We should probably also add a test.

We don't change the behavior of the allocator. However, the original CHECK prohibited any resource removal, so we need to change the CHECK. The newly introduced variable is just for rewriting the new CHECK.

Since this is a CHECK, it would not be easy to test the violation. But I'll add an allocator test to make sure that the CHECK passes when we reduce resources.


- Chun-Hung


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


On July 10, 2018, 5:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated July 10, 2018, 5:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-9015
>     https://issues.apache.org/jira/browse/MESOS-9015
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch allows the allocator to remove resources when calling
> `Sorter::update`, so Mesos can remove resources that are no longer
> available through resource conversions.
> 
> An example usage is that we can make `DESTROY_VOLUME` to convert a
> volume with a stale profile to an empty resource, so the disk space
> will disappear instead of being offered out to other frameworks.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
>   docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
>   src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
>   src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
>   src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/4/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On July 10, 2018, 3:33 p.m., Benjamin Bannier wrote:
> > src/tests/sorter_tests.cpp
> > Lines 1317-1318 (patched)
> > <https://reviews.apache.org/r/67669/diff/4/?file=2056898#file2056898line1317>
> >
> >     Not sure we really need two resource kinds here. For me it just seems to make this test harder without testing anything interesting.
> >     
> >     I'll leave this up to you.

Good point. I'll update it.


- Chun-Hung


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


On July 10, 2018, 5:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated July 10, 2018, 5:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-9015
>     https://issues.apache.org/jira/browse/MESOS-9015
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch allows the allocator to remove resources when calling
> `Sorter::update`, so Mesos can remove resources that are no longer
> available through resource conversions.
> 
> An example usage is that we can make `DESTROY_VOLUME` to convert a
> volume with a stale profile to an empty resource, so the disk space
> will disappear instead of being offered out to other frameworks.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
>   docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
>   src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
>   src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
>   src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/4/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On July 10, 2018, 3:33 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 924-926 (original), 934-939 (patched)
> > <https://reviews.apache.org/r/67669/diff/4/?file=2056895#file2056895line936>
> >
> >     I believe we do not need the potentially expensive calls to `toUnreserved` here as `createStrippedScalarQuantity` creates unreserved resources.

You're right. The `createStrippedScalarQuantity()` implementation is changed recently. Previously we keep the static reservation and thus the additional `toUnreserved()` call.


- Chun-Hung


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


On July 10, 2018, 5:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated July 10, 2018, 5:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-9015
>     https://issues.apache.org/jira/browse/MESOS-9015
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch allows the allocator to remove resources when calling
> `Sorter::update`, so Mesos can remove resources that are no longer
> available through resource conversions.
> 
> An example usage is that we can make `DESTROY_VOLUME` to convert a
> volume with a stale profile to an empty resource, so the disk space
> will disappear instead of being offered out to other frameworks.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
>   docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
>   src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
>   src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
>   src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/4/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67669/#review205911
-----------------------------------------------------------




CHANGELOG
Lines 28 (patched)
<https://reviews.apache.org/r/67669/#comment288832>

    Let's use this issue link for this RR.



docs/upgrades.md
Lines 469 (patched)
<https://reviews.apache.org/r/67669/#comment288833>

    I don't think we do need to mention any operations. What about e.g.,
    
    > The semantics of `Sorter::update` has been changed so that resources can be removed certain agents in the pool without removing the full agent. Users are expected to update the total resources as well by e.g., removing the old and adding back the new resources for an agent.



src/master/allocator/mesos/hierarchical.cpp
Lines 887 (patched)
<https://reviews.apache.org/r/67669/#comment288837>

    Why are changes to the allocator in this file? Should they go somewhere else where we enable a feature or explicitly fix a bug? We should probably also add a test.



src/master/allocator/mesos/hierarchical.cpp
Lines 924-926 (original), 934-939 (patched)
<https://reviews.apache.org/r/67669/#comment288835>

    I believe we do not need the potentially expensive calls to `toUnreserved` here as `createStrippedScalarQuantity` creates unreserved resources.



src/tests/sorter_tests.cpp
Lines 1317-1318 (patched)
<https://reviews.apache.org/r/67669/#comment288836>

    Not sure we really need two resource kinds here. For me it just seems to make this test harder without testing anything interesting.
    
    I'll leave this up to you.


- Benjamin Bannier


On July 10, 2018, 5:39 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated July 10, 2018, 5:39 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-8995
>     https://issues.apache.org/jira/browse/MESOS-8995
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch allows the allocator to remove resources when calling
> `Sorter::update`, so Mesos can remove resources that are no longer
> available through resource conversions.
> 
> An example usage is that we can make `DESTROY_VOLUME` to convert a
> volume with a stale profile to an empty resource, so the disk space
> will disappear instead of being offered out to other frameworks.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
>   docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
>   src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
>   src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
>   src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/4/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Allowed resources to be removed when updating the sorter.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67669/#review206068
-----------------------------------------------------------


Ship it!




Ship It!

- Jie Yu


On July 10, 2018, 8:31 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67669/
> -----------------------------------------------------------
> 
> (Updated July 10, 2018, 8:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.
> 
> 
> Bugs: MESOS-9015
>     https://issues.apache.org/jira/browse/MESOS-9015
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch allows the allocator to remove resources when calling
> `Sorter::update`, so Mesos can remove resources that are no longer
> available through resource conversions.
> 
> An example usage is that we can make `DESTROY_VOLUME` to convert a
> volume with a stale profile to an empty resource, so the disk space
> will disappear instead of being offered out to other frameworks.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
>   docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
>   src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
>   src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
>   src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 
> 
> 
> Diff: https://reviews.apache.org/r/67669/diff/5/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 67669: Allowed resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67669/
-----------------------------------------------------------

(Updated July 10, 2018, 8:31 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.


Changes
-------

Addressed Benjamin's comments.


Summary (updated)
-----------------

Allowed resources to be removed when updating the sorter.


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


Repository: mesos


Description
-------

This patch allows the allocator to remove resources when calling
`Sorter::update`, so Mesos can remove resources that are no longer
available through resource conversions.

An example usage is that we can make `DESTROY_VOLUME` to convert a
volume with a stale profile to an empty resource, so the disk space
will disappear instead of being offered out to other frameworks.


Diffs (updated)
-----

  CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
  docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
  src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
  src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
  src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 


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

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


Testing
-------

sudo make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67669/
-----------------------------------------------------------

(Updated July 10, 2018, 5:58 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.


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


Repository: mesos


Description
-------

This patch allows the allocator to remove resources when calling
`Sorter::update`, so Mesos can remove resources that are no longer
available through resource conversions.

An example usage is that we can make `DESTROY_VOLUME` to convert a
volume with a stale profile to an empty resource, so the disk space
will disappear instead of being offered out to other frameworks.


Diffs
-----

  CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
  docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
  src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
  src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
  src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
  src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 


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


Testing
-------

sudo make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67669/
-----------------------------------------------------------

(Updated July 10, 2018, 3:39 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.


Changes
-------

Fixed a bug in the test.


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


Repository: mesos


Description
-------

This patch allows the allocator to remove resources when calling
`Sorter::update`, so Mesos can remove resources that are no longer
available through resource conversions.

An example usage is that we can make `DESTROY_VOLUME` to convert a
volume with a stale profile to an empty resource, so the disk space
will disappear instead of being offered out to other frameworks.


Diffs (updated)
-----

  CHANGELOG 4aef45a28c2127587b003dc0a5e2855ac108e6ff 
  docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
  src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
  src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
  src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
  src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 


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

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


Testing
-------

sudo make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 67669: Allow resources to be removed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67669/
-----------------------------------------------------------

(Updated July 6, 2018, 11:27 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.


Changes
-------

Keep the check but only relex it to support MESOS-8825.


Summary (updated)
-----------------

Allow resources to be removed when updating the sorter.


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


Repository: mesos


Description (updated)
-------

This patch allows the allocator to remove resources when calling
`Sorter::update`, so Mesos can remove resources that are no longer
available through resource conversions.

An example usage is that we can make `DESTROY_VOLUME` to convert a
volume with a stale profile to an empty resource, so the disk space
will disappear instead of being offered out to other frameworks.


Diffs (updated)
-----

  CHANGELOG d0cc0e0a4d2a7aa69cadc21923c7af397ebcbb25 
  docs/upgrades.md cc8a251945af27a432dbeb5585b998cd84e35b22 
  src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
  src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
  src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
  src/tests/sorter_tests.cpp 266a9e72597b3aadc9756f61736c0adf0b1a5831 


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

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


Testing
-------

sudo make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 67669: Allowed allocation quantities to be changed when updating the sorter.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67669/
-----------------------------------------------------------

(Updated June 21, 2018, 4:52 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng Zhu.


Changes
-------

Partially addressed Benjamin's comment.


Summary (updated)
-----------------

Allowed allocation quantities to be changed when updating the sorter.


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


Repository: mesos


Description (updated)
-------

This patch allows the allocator to change allocation quantities when
calling `Sorter::update`, so Mesos can support arbitrary resource
conversions.

An example usage is that we can make `DESTROY_VOLUME` to convert a
volume with a stale profile to an empty resource, so the disk space
will disappear instead of being offered out to other frameworks.


Diffs (updated)
-----

  CHANGELOG 0e44277389e5a96c221d6c3d501da2bb93d27faa 
  docs/upgrades.md cc8a251945af27a432dbeb5585b998cd84e35b22 
  src/master/allocator/mesos/hierarchical.cpp b558228290e5ae6dbcc1b8a6e1fe69db9fd5874c 
  src/master/allocator/sorter/drf/sorter.cpp 755a36ead42a91053c881fc46dc6acb39c819cac 
  src/master/allocator/sorter/sorter.hpp e5e86513dc4023d9c471753897635923c6c9203f 
  src/tests/sorter_tests.cpp 9cdffd778967bca3d0055be8a04fbb702432e59d 


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

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


Testing
-------

sudo make check


Thanks,

Chun-Hung Hsiao