You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2019/05/28 14:55:21 UTC

Review Request 70737: Added a method to shrink `Resources` to target `ResourceLimits`.

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

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


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


Repository: mesos


Description
-------

Also added unit tests.


Diffs
-----

  include/mesos/resources.hpp 609ca61a0ffa80a7b10e6a3ebb0caa15966c3371 
  src/common/resources.cpp 543820de435eb4869fbb93534fa00c58a9c5dfdd 
  src/tests/resources_tests.cpp dbae0f6d7832a941cd04f3d76493f3e486c3909b 


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


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 70737: Added a method to shrink `Resources` to target `ResourceLimits`.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70737/#review215542
-----------------------------------------------------------



Let's keep this out of the resources class to start with, much like we did with the original shrink function. Unless there are some clear non-allocator use cases for it?

As it stands, the randomness make it seem inappropriate as a general Resources member function.


include/mesos/resources.hpp
Lines 552-565 (original), 557-583 (patched)
<https://reviews.apache.org/r/70737/#comment302270>

    What's the difference between these two functions? I would suggest documenting them together as one, with some clarity about how they're different. Rather than duplicating the same documentation and leaving it to the reader to figure out the subtle difference.


- Benjamin Mahler


On May 28, 2019, 2:55 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70737/
> -----------------------------------------------------------
> 
> (Updated May 28, 2019, 2:55 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8456
>     https://issues.apache.org/jira/browse/MESOS-8456
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Also added unit tests.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp 609ca61a0ffa80a7b10e6a3ebb0caa15966c3371 
>   src/common/resources.cpp 543820de435eb4869fbb93534fa00c58a9c5dfdd 
>   src/tests/resources_tests.cpp dbae0f6d7832a941cd04f3d76493f3e486c3909b 
> 
> 
> Diff: https://reviews.apache.org/r/70737/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70737: Added a function to shrink `Resources` to target `ResourceLimits`.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70737/#review215591
-----------------------------------------------------------


Ship it!




Ditto here, ideally this could be an allocator specific header

- Benjamin Mahler


On May 28, 2019, 2:55 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70737/
> -----------------------------------------------------------
> 
> (Updated May 28, 2019, 2:55 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8456
>     https://issues.apache.org/jira/browse/MESOS-8456
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Also added unit tests.
> 
> 
> Diffs
> -----
> 
>   src/common/resources_utils.hpp 42b6cafa3dbcdcce67690dd359c4aecb517ffd1d 
>   src/common/resources_utils.cpp 3786bcf3ab61048c3a92e41acb4e43fd787d54f8 
>   src/tests/resources_tests.cpp dbae0f6d7832a941cd04f3d76493f3e486c3909b 
> 
> 
> Diff: https://reviews.apache.org/r/70737/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70737: Added a function to shrink `Resources` to target `ResourceLimits`.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70737/#review215598
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70750, 70707, 70713, 70716, 70730, 70731, 70733, 70734, 70735, 70737]

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

- Mesos Reviewbot


On May 28, 2019, 2:55 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70737/
> -----------------------------------------------------------
> 
> (Updated May 28, 2019, 2:55 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8456
>     https://issues.apache.org/jira/browse/MESOS-8456
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Also added unit tests.
> 
> 
> Diffs
> -----
> 
>   src/common/resources_utils.hpp 42b6cafa3dbcdcce67690dd359c4aecb517ffd1d 
>   src/common/resources_utils.cpp 3786bcf3ab61048c3a92e41acb4e43fd787d54f8 
>   src/tests/resources_tests.cpp dbae0f6d7832a941cd04f3d76493f3e486c3909b 
> 
> 
> Diff: https://reviews.apache.org/r/70737/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70737: Added a method to shrink `Resources` to target `ResourceLimits`.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70737/#review215541
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70705, 70706, 70707, 70713, 70716, 70730, 70731, 70733, 70734, 70735, 70737]

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

- Mesos Reviewbot


On May 28, 2019, 2:55 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70737/
> -----------------------------------------------------------
> 
> (Updated May 28, 2019, 2:55 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8456
>     https://issues.apache.org/jira/browse/MESOS-8456
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Also added unit tests.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp 609ca61a0ffa80a7b10e6a3ebb0caa15966c3371 
>   src/common/resources.cpp 543820de435eb4869fbb93534fa00c58a9c5dfdd 
>   src/tests/resources_tests.cpp dbae0f6d7832a941cd04f3d76493f3e486c3909b 
> 
> 
> Diff: https://reviews.apache.org/r/70737/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>