mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chun-Hung Hsiao <chhs...@apache.org>
Subject Re: Review Request 67669: Allow resources to be removed when updating the sorter.
Date Tue, 10 Jul 2018 20:30:07 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message