You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2020/04/21 13:18:34 UTC
Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/
-----------------------------------------------------------
Review request for mesos, Andrei Budnik and Greg Mann.
Bugs: MESOS-10117
https://issues.apache.org/jira/browse/MESOS-10117
Repository: mesos
Description
-------
Updated UCR's `usage()` method to support resource limits.
Diffs
-----
src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
Diff: https://reviews.apache.org/r/72399/diff/1/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Qian Zhang <zh...@gmail.com>.
> On April 29, 2020, 3:01 a.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 2517-2519 (original), 2589-2594 (patched)
> > <https://reviews.apache.org/r/72399/diff/5/?file=2227708#file2227708line2592>
> >
> > This stuff looks stale to me, do you think we can remove it?
No, we still have this issue actually, after recovery both `resourceRequests` and `resourceLimits` of each `Container` object in UCR are empty until `MesosContainerizerProcess::update()` is called.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/#review220528
-----------------------------------------------------------
On April 29, 2020, 7:53 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72399/
> -----------------------------------------------------------
>
> (Updated April 29, 2020, 7:53 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10117
> https://issues.apache.org/jira/browse/MESOS-10117
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated UCR's `usage()` method to support resource limits.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
> src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
> src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
> src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
>
>
> Diff: https://reviews.apache.org/r/72399/diff/6/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/#review220528
-----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp
Lines 2460-2463 (original), 2459-2465 (patched)
<https://reviews.apache.org/r/72399/#comment309008>
Instead of having both the `containerConfig` argument as well as `resourceRequests`/`resourceLimits`, couldn't we get rid of `containerConfig` and use a conditional check for `if (containerId.has_parent())` in `usage()` to control whether we get the request/limit values from the `ContainerConfig` or from the `Container` struct?
src/slave/containerizer/mesos/containerizer.cpp
Lines 2481-2486 (original), 2483-2508 (patched)
<https://reviews.apache.org/r/72399/#comment309009>
Can we move this logic into the conditionals for `if (cpuLimit.isSome())` and `if (memLimit.isSome())` below?
src/slave/containerizer/mesos/containerizer.cpp
Lines 2517 (patched)
<https://reviews.apache.org/r/72399/#comment309010>
Nit: make `resources` a const ref?
src/slave/containerizer/mesos/containerizer.cpp
Lines 2533-2538 (patched)
<https://reviews.apache.org/r/72399/#comment309011>
Do you want to move this `foreach` loop out of the conditional, since it's shared by both branches?
src/slave/containerizer/mesos/containerizer.cpp
Lines 2517-2519 (original), 2589-2594 (patched)
<https://reviews.apache.org/r/72399/#comment309007>
This stuff looks stale to me, do you think we can remove it?
- Greg Mann
On April 25, 2020, 10:55 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72399/
> -----------------------------------------------------------
>
> (Updated April 25, 2020, 10:55 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10117
> https://issues.apache.org/jira/browse/MESOS-10117
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated UCR's `usage()` method to support resource limits.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
> src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
> src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
> src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
>
>
> Diff: https://reviews.apache.org/r/72399/diff/5/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/
-----------------------------------------------------------
(Updated May 5, 2020, 4:11 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Addressed review comments.
Bugs: MESOS-10117
https://issues.apache.org/jira/browse/MESOS-10117
Repository: mesos
Description
-------
Updated UCR's `usage()` method to support resource limits.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
Diff: https://reviews.apache.org/r/72399/diff/7/
Changes: https://reviews.apache.org/r/72399/diff/6-7/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/#review220602
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/containerizer/mesos/containerizer.cpp
Lines 2587 (patched)
<https://reviews.apache.org/r/72399/#comment309099>
s/too//
- Greg Mann
On April 29, 2020, 11:53 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72399/
> -----------------------------------------------------------
>
> (Updated April 29, 2020, 11:53 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10117
> https://issues.apache.org/jira/browse/MESOS-10117
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated UCR's `usage()` method to support resource limits.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
> src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
> src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
> src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
>
>
> Diff: https://reviews.apache.org/r/72399/diff/6/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/
-----------------------------------------------------------
(Updated April 29, 2020, 7:53 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Addressed review comments.
Bugs: MESOS-10117
https://issues.apache.org/jira/browse/MESOS-10117
Repository: mesos
Description
-------
Updated UCR's `usage()` method to support resource limits.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
Diff: https://reviews.apache.org/r/72399/diff/6/
Changes: https://reviews.apache.org/r/72399/diff/5-6/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/
-----------------------------------------------------------
(Updated April 25, 2020, 6:55 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Set container's hard CPU limit to host total CPU numbers if the CPU limit specified by framework is infinity.
Bugs: MESOS-10117
https://issues.apache.org/jira/browse/MESOS-10117
Repository: mesos
Description
-------
Updated UCR's `usage()` method to support resource limits.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
Diff: https://reviews.apache.org/r/72399/diff/5/
Changes: https://reviews.apache.org/r/72399/diff/4-5/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/
-----------------------------------------------------------
(Updated April 24, 2020, 8:25 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Set container's hard memory limit to host total memory if the memory limit specified by framework is infinity.
Bugs: MESOS-10117
https://issues.apache.org/jira/browse/MESOS-10117
Repository: mesos
Description
-------
Updated UCR's `usage()` method to support resource limits.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
Diff: https://reviews.apache.org/r/72399/diff/4/
Changes: https://reviews.apache.org/r/72399/diff/3-4/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/
-----------------------------------------------------------
(Updated April 22, 2020, 5:30 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Fixed a minor issue in test.
Bugs: MESOS-10117
https://issues.apache.org/jira/browse/MESOS-10117
Repository: mesos
Description
-------
Updated UCR's `usage()` method to support resource limits.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
Diff: https://reviews.apache.org/r/72399/diff/3/
Changes: https://reviews.apache.org/r/72399/diff/2-3/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72399: Updated UCR's `usage()` method to support
resource limits.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72399/
-----------------------------------------------------------
(Updated April 22, 2020, 4:01 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Took the agent flag `--cgroups_enable_cfs` into account.
Bugs: MESOS-10117
https://issues.apache.org/jira/browse/MESOS-10117
Repository: mesos
Description
-------
Updated UCR's `usage()` method to support resource limits.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 2ea033aa1869f07644b10c036fda1698d08aa89b
src/slave/containerizer/mesos/containerizer.cpp 6aa4f3fe0940575aeea6a63cdb1ca3c77cd2359b
src/tests/slave_recovery_tests.cpp 0efd3a6ac09ad06d9365b7bb2295157b5175e6b8
src/tests/slave_tests.cpp 6b264d067accb9800ba75ea8f5d27c1e4c5593db
Diff: https://reviews.apache.org/r/72399/diff/2/
Changes: https://reviews.apache.org/r/72399/diff/1-2/
Testing
-------
Thanks,
Qian Zhang