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/02/26 12:19:49 UTC

Re: Review Request 71944: Set container process's OOM score adjust.

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

(Updated Feb. 26, 2020, 8:19 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
-------

Changed `totalMem` from member varaible to a static variable.


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


Repository: mesos


Description
-------

Set container process's OOM score adjust.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp b12b73d8e0161d448075378765e77867521de04e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp a311ab4495f71bedacd2e99c84c765f0e5fe99d3 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp dc6c7aa1c998c30c8b17db04a38e7a1e28a6a6c1 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp c62deec4b1cd749dba5fe71b901e0353806a0805 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp ac2e66b570bb84b43c4a3e8f19b40e5fcea71a4a 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 27d88e91fb784179effd54781f84000fe85c13eb 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 06531072f445d4ec978ebaf5ec5e4a2475517d05 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp ec2ce67e54387f26aa11c00d4c7f85f0807a127b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 2c865aca35084a5db567b5f95c8c57bb6e1d5634 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 180afc936798c2fa4de0deef080276cf7cc94199 


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

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


Testing
-------

sudo make check


Thanks,

Qian Zhang


Re: Review Request 71944: Set container process's OOM score adjust.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71944/#review219991
-----------------------------------------------------------


Ship it!




Ship It!

- Greg Mann


On March 16, 2020, 9:10 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71944/
> -----------------------------------------------------------
> 
> (Updated March 16, 2020, 9:10 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10048
>     https://issues.apache.org/jira/browse/MESOS-10048
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Set container process's OOM score adjust.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp bf2a4d8d587969568f451f63e4f619e9c49f3642 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp a311ab4495f71bedacd2e99c84c765f0e5fe99d3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp dc6c7aa1c998c30c8b17db04a38e7a1e28a6a6c1 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp c62deec4b1cd749dba5fe71b901e0353806a0805 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp ac2e66b570bb84b43c4a3e8f19b40e5fcea71a4a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 27d88e91fb784179effd54781f84000fe85c13eb 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 06531072f445d4ec978ebaf5ec5e4a2475517d05 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp ec2ce67e54387f26aa11c00d4c7f85f0807a127b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 2c865aca35084a5db567b5f95c8c57bb6e1d5634 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 180afc936798c2fa4de0deef080276cf7cc94199 
>   src/slave/containerizer/mesos/utils.hpp bfd07e28c78fc140e395ffccd11d65545bf007fc 
>   src/slave/containerizer/mesos/utils.cpp d9964f0b0e8ad8a41e5f5490a1c28bba9a972ae0 
> 
> 
> Diff: https://reviews.apache.org/r/71944/diff/8/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 71944: Set container process's OOM score adjust.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71944/
-----------------------------------------------------------

(Updated March 16, 2020, 5:10 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Set container process's OOM score adjust.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp bf2a4d8d587969568f451f63e4f619e9c49f3642 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp a311ab4495f71bedacd2e99c84c765f0e5fe99d3 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp dc6c7aa1c998c30c8b17db04a38e7a1e28a6a6c1 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp c62deec4b1cd749dba5fe71b901e0353806a0805 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp ac2e66b570bb84b43c4a3e8f19b40e5fcea71a4a 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 27d88e91fb784179effd54781f84000fe85c13eb 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 06531072f445d4ec978ebaf5ec5e4a2475517d05 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp ec2ce67e54387f26aa11c00d4c7f85f0807a127b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 2c865aca35084a5db567b5f95c8c57bb6e1d5634 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 180afc936798c2fa4de0deef080276cf7cc94199 
  src/slave/containerizer/mesos/utils.hpp bfd07e28c78fc140e395ffccd11d65545bf007fc 
  src/slave/containerizer/mesos/utils.cpp d9964f0b0e8ad8a41e5f5490a1c28bba9a972ae0 


Diff: https://reviews.apache.org/r/71944/diff/8/

Changes: https://reviews.apache.org/r/71944/diff/7-8/


Testing
-------

sudo make check


Thanks,

Qian Zhang


Re: Review Request 71944: Set container process's OOM score adjust.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71944/
-----------------------------------------------------------

(Updated March 16, 2020, 3:17 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
-------

Updated some comments in the code.


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


Repository: mesos


Description
-------

Set container process's OOM score adjust.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp bf2a4d8d587969568f451f63e4f619e9c49f3642 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp a311ab4495f71bedacd2e99c84c765f0e5fe99d3 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp dc6c7aa1c998c30c8b17db04a38e7a1e28a6a6c1 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp c62deec4b1cd749dba5fe71b901e0353806a0805 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp ac2e66b570bb84b43c4a3e8f19b40e5fcea71a4a 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 27d88e91fb784179effd54781f84000fe85c13eb 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 06531072f445d4ec978ebaf5ec5e4a2475517d05 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp ec2ce67e54387f26aa11c00d4c7f85f0807a127b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 2c865aca35084a5db567b5f95c8c57bb6e1d5634 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 180afc936798c2fa4de0deef080276cf7cc94199 
  src/slave/containerizer/mesos/utils.hpp bfd07e28c78fc140e395ffccd11d65545bf007fc 
  src/slave/containerizer/mesos/utils.cpp d9964f0b0e8ad8a41e5f5490a1c28bba9a972ae0 


Diff: https://reviews.apache.org/r/71944/diff/7/

Changes: https://reviews.apache.org/r/71944/diff/6-7/


Testing
-------

sudo make check


Thanks,

Qian Zhang


Re: Review Request 71944: Set container process's OOM score adjust.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71944/
-----------------------------------------------------------

(Updated March 11, 2020, 5:40 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
-------

Addressed review comments.


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


Repository: mesos


Description
-------

Set container process's OOM score adjust.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp b12b73d8e0161d448075378765e77867521de04e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp a311ab4495f71bedacd2e99c84c765f0e5fe99d3 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp dc6c7aa1c998c30c8b17db04a38e7a1e28a6a6c1 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp c62deec4b1cd749dba5fe71b901e0353806a0805 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp ac2e66b570bb84b43c4a3e8f19b40e5fcea71a4a 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 27d88e91fb784179effd54781f84000fe85c13eb 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 06531072f445d4ec978ebaf5ec5e4a2475517d05 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp ec2ce67e54387f26aa11c00d4c7f85f0807a127b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 2c865aca35084a5db567b5f95c8c57bb6e1d5634 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 180afc936798c2fa4de0deef080276cf7cc94199 
  src/slave/containerizer/mesos/utils.hpp bfd07e28c78fc140e395ffccd11d65545bf007fc 
  src/slave/containerizer/mesos/utils.cpp d9964f0b0e8ad8a41e5f5490a1c28bba9a972ae0 


Diff: https://reviews.apache.org/r/71944/diff/6/

Changes: https://reviews.apache.org/r/71944/diff/5-6/


Testing
-------

sudo make check


Thanks,

Qian Zhang


Re: Review Request 71944: Set container process's OOM score adjust.

Posted by Qian Zhang <zh...@gmail.com>.

> On March 2, 2020, 9:50 p.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp
> > Lines 222 (patched)
> > <https://reviews.apache.org/r/71944/diff/5/?file=2212178#file2212178line222>
> >
> >     Why don't we want to set the OOM score adjustment for custom executor containers?

For custom executor (actually same to default executor), if its soft limit < hard limit, that means it must launch some nested containers whose `share_cgroups` is false and soft limit < hard limit, in which case we will set OOM score adjustment for each nested container, and we assume the customer executor itself will not consume much memory, so we just leave its OOM score adjustment as 0.

If the tasks launched by custom executor have `share_cgroups` as true, then custom executor's soft limit must be equal to its hard limit (since no resource limits can be specified for those tasks), so here we will still leave custom executor's OOM score adjustment as 0, and this ensures backward compatibility.


- Qian


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


On March 11, 2020, 5:40 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71944/
> -----------------------------------------------------------
> 
> (Updated March 11, 2020, 5:40 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10048
>     https://issues.apache.org/jira/browse/MESOS-10048
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Set container process's OOM score adjust.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp b12b73d8e0161d448075378765e77867521de04e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp a311ab4495f71bedacd2e99c84c765f0e5fe99d3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp dc6c7aa1c998c30c8b17db04a38e7a1e28a6a6c1 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp c62deec4b1cd749dba5fe71b901e0353806a0805 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp ac2e66b570bb84b43c4a3e8f19b40e5fcea71a4a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 27d88e91fb784179effd54781f84000fe85c13eb 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 06531072f445d4ec978ebaf5ec5e4a2475517d05 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp ec2ce67e54387f26aa11c00d4c7f85f0807a127b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 2c865aca35084a5db567b5f95c8c57bb6e1d5634 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 180afc936798c2fa4de0deef080276cf7cc94199 
>   src/slave/containerizer/mesos/utils.hpp bfd07e28c78fc140e395ffccd11d65545bf007fc 
>   src/slave/containerizer/mesos/utils.cpp d9964f0b0e8ad8a41e5f5490a1c28bba9a972ae0 
> 
> 
> Diff: https://reviews.apache.org/r/71944/diff/6/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 71944: Set container process's OOM score adjust.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71944/#review219718
-----------------------------------------------------------




src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp
Lines 222 (patched)
<https://reviews.apache.org/r/71944/#comment307924>

    Why don't we want to set the OOM score adjustment for custom executor containers?



src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp
Lines 233-244 (patched)
<https://reviews.apache.org/r/71944/#comment307925>

    It looks like you could get rid of this conditional for `if (oomScoreAdj.isSome())`, make `oomScoreAdj` a raw integer, and do all of this work within the conditional directly above this?


- Greg Mann


On Feb. 26, 2020, 12:19 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71944/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2020, 12:19 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10048
>     https://issues.apache.org/jira/browse/MESOS-10048
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Set container process's OOM score adjust.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp b12b73d8e0161d448075378765e77867521de04e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp a311ab4495f71bedacd2e99c84c765f0e5fe99d3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp dc6c7aa1c998c30c8b17db04a38e7a1e28a6a6c1 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp c62deec4b1cd749dba5fe71b901e0353806a0805 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp ac2e66b570bb84b43c4a3e8f19b40e5fcea71a4a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 27d88e91fb784179effd54781f84000fe85c13eb 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 06531072f445d4ec978ebaf5ec5e4a2475517d05 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp ec2ce67e54387f26aa11c00d4c7f85f0807a127b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 2c865aca35084a5db567b5f95c8c57bb6e1d5634 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 180afc936798c2fa4de0deef080276cf7cc94199 
> 
> 
> Diff: https://reviews.apache.org/r/71944/diff/5/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>