You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/05/01 21:23:59 UTC
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated May 1, 2014, 12:23 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Jie Yu, and Vinod Kone.
Changes
-------
Rebased.
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs (updated)
-----
src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review45058
-----------------------------------------------------------
Bad patch!
Reviews applied: [20818]
Failed command: git apply --index 20818.patch
Error:
error: patch failed: src/linux/cgroups.cpp:1439
error: src/linux/cgroups.cpp: patch does not apply
- Mesos ReviewBot
On June 9, 2014, 4:15 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated June 9, 2014, 4:15 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
> src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review45541
-----------------------------------------------------------
src/linux/cgroups.hpp
<https://reviews.apache.org/r/20818/#comment80388>
Why this function is still here?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment80389>
I would prefer terminate the process immediately.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment80395>
Let's fail the promise here as 'future' is not expected to be discarded:
promise.fail("Not expecting discarded future");
terminate(self());
return
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment80396>
Ditto above. Let's move the logic to 'finalize'
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment80397>
Ditto
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment80398>
ditto.
- Jie Yu
On June 12, 2014, 5:24 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated June 12, 2014, 5:24 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
> src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review45587
-----------------------------------------------------------
Bad patch!
Reviews applied: [20817, 20818]
Failed command: ./support/mesos-style.py
Error:
Checking 470 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo
src/linux/cgroups.cpp:1431: Do not leave a blank line after "private:" [whitespace/blank_line] [3]
Total errors found: 1
- Mesos ReviewBot
On June 12, 2014, 5:24 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated June 12, 2014, 5:24 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
> src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review45949
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On June 16, 2014, 9:58 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated June 16, 2014, 9:58 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.cpp eb8933f4e37e09e52123b03890b6df952ceb6ec8
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
> src/slave/containerizer/mesos_containerizer.cpp d6df9a1e41024102f3a63cda1b1868b641386172
> src/tests/cgroups_tests.cpp adcf46b1a4071b8f29475d2e20aae6dd8fdf09e7
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated June 16, 2014, 2:58 p.m.)
Review request for mesos and Jie Yu.
Bugs: MESOS-759 and MESOS-976
https://issues.apache.org/jira/browse/MESOS-759
https://issues.apache.org/jira/browse/MESOS-976
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs (updated)
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.cpp eb8933f4e37e09e52123b03890b6df952ceb6ec8
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
src/slave/containerizer/mesos_containerizer.cpp d6df9a1e41024102f3a63cda1b1868b641386172
src/tests/cgroups_tests.cpp adcf46b1a4071b8f29475d2e20aae6dd8fdf09e7
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated June 12, 2014, 10:24 a.m.)
Review request for mesos and Jie Yu.
Bugs: MESOS-759 and MESOS-976
https://issues.apache.org/jira/browse/MESOS-759
https://issues.apache.org/jira/browse/MESOS-976
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review45509
-----------------------------------------------------------
Bad patch!
Reviews applied: [20818]
Failed command: git apply --index 20818.patch
Error:
error: patch failed: src/linux/cgroups.hpp:41
error: src/linux/cgroups.hpp: patch does not apply
error: patch failed: src/linux/cgroups.cpp:1435
error: src/linux/cgroups.cpp: patch does not apply
- Mesos ReviewBot
On June 11, 2014, 10:21 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated June 11, 2014, 10:21 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
> src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated June 11, 2014, 3:21 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
Terminate immediately on discard.
Bugs: MESOS-759 and MESOS-976
https://issues.apache.org/jira/browse/MESOS-759
https://issues.apache.org/jira/browse/MESOS-976
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs (updated)
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated June 11, 2014, 10:27 a.m.)
Review request for mesos and Jie Yu.
Changes
-------
Added depends on.
Bugs: MESOS-759 and MESOS-976
https://issues.apache.org/jira/browse/MESOS-759
https://issues.apache.org/jira/browse/MESOS-976
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated June 8, 2014, 9:15 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
Rebased.
Bugs: MESOS-759 and MESOS-976
https://issues.apache.org/jira/browse/MESOS-759
https://issues.apache.org/jira/browse/MESOS-976
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs (updated)
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review44398
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On May 29, 2014, 10:26 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated May 29, 2014, 10:26 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
> src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated May 29, 2014, 3:26 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
Addressed Chi's comments.
Bugs: MESOS-759 and MESOS-976
https://issues.apache.org/jira/browse/MESOS-759
https://issues.apache.org/jira/browse/MESOS-976
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs (updated)
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review43511
-----------------------------------------------------------
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment77699>
This comment isn't quite useful. Can probably borrow the idea from EmptyWatcher?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment77700>
dido
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment77703>
also change CHECK to if check, like you did in TaskKiller?
- Chi Zhang
On May 15, 2014, 10:24 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated May 15, 2014, 10:24 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
> src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
> src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
> src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
> src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated May 15, 2014, 10:24 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
assigning @jieyu as shepherd -- vinod
Bugs: MESOS-759 and MESOS-976
https://issues.apache.org/jira/browse/MESOS-759
https://issues.apache.org/jira/browse/MESOS-976
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs
-----
src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
> On May 14, 2014, 10:16 a.m., Jie Yu wrote:
> > src/linux/cgroups.cpp, lines 1634-1645
> > <https://reviews.apache.org/r/20818/diff/4/?file=572874#file572874line1634>
> >
> > This sequence of operations are introduced in this commit. Probably you should contact the author to see if your current logic is OK. If yes, please add some comments to explain why it is OK.
> >
> > commit 77db3cb32f9d25656b86607f6f241b2303dbd982
> > Author: Brenden Matthews <br...@airbnb.com>
> > Date: Fri May 10 16:00:15 2013 -0700
> >
> > Changed cgroups killTasks() sequence.
> >
> > The sequence is as follows:
> > stop -> kill -> empty -> freeze -> kill -> thaw -> empty
> >
> > We also now ignore ESRCH errors from kill() in cgroups::kill().
> >
> > Review: https://reviews.apache.org/r/11131
Brendan commented that this was introduced when it was observed that OOM'ed processes weren't being killed successfully.
At this time the cgroups_isolator was disabling the kernel OOM killer and trying to handle the OOM in Mesos. Trying to handle this in userspace is a bad idea and could lead to processes stuck in weird states. This is no longer done in Mesos and the kernel OOM killer is used.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review42977
-----------------------------------------------------------
On May 5, 2014, 1:16 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 1:16 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
> src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
> src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
> src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
> src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review42977
-----------------------------------------------------------
src/linux/cgroups.hpp
<https://reviews.apache.org/r/20818/#comment76956>
It's probably fine for this review. But in the future, we should try to push timeout to higher level if possible.
For example, one can discard the future returned by 'destroy' to cancel the operation and use future.after to setup timeout:
destroy(...).after(Seconds(60), [](...) {
future.discard();
return Failure(...);
});
src/linux/cgroups.hpp
<https://reviews.apache.org/r/20818/#comment76958>
Use 'Timeout' here, instead of Duration, given that Freezer uses Timeout?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment76955>
This sequence of operations are introduced in this commit. Probably you should contact the author to see if your current logic is OK. If yes, please add some comments to explain why it is OK.
commit 77db3cb32f9d25656b86607f6f241b2303dbd982
Author: Brenden Matthews <br...@airbnb.com>
Date: Fri May 10 16:00:15 2013 -0700
Changed cgroups killTasks() sequence.
The sequence is as follows:
stop -> kill -> empty -> freeze -> kill -> thaw -> empty
We also now ignore ESRCH errors from kill() in cgroups::kill().
Review: https://reviews.apache.org/r/11131
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment76959>
So the 'timeout' specified by the user is not the total timeout for the operation?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment76960>
Do you need to discard the future here:
future.discard()?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment76961>
Ditto.
- Jie Yu
On May 5, 2014, 8:16 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 8:16 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
> src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
> src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
> src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
> src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
> On May 5, 2014, 4:18 p.m., Vinod Kone wrote:
> > src/linux/cgroups.cpp, lines 1713-1715
> > <https://reviews.apache.org/r/20818/diff/4/?file=572874#file572874line1713>
> >
> > How come we are not doing retires anymore?
Retries are done for freeze and thaw. See additional comments below in response to Jie's question about the SIGSTOP logic.
> On May 5, 2014, 4:18 p.m., Vinod Kone wrote:
> > src/linux/cgroups.cpp, lines 1636-1638
> > <https://reviews.apache.org/r/20818/diff/4/?file=572874#file572874line1636>
> >
> > IIRC, we added SIGSTOP and SIGKILL before freeze because the earlier freeze, kill, thaw sequence didn't always work. Have the semantics changed so that this is guaranteed to work?
See response to Jie's comment, below.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review42216
-----------------------------------------------------------
On May 5, 2014, 1:16 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 1:16 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
> src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
> src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
> src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
> src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review42216
-----------------------------------------------------------
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment75993>
IIRC, we added SIGSTOP and SIGKILL before freeze because the earlier freeze, kill, thaw sequence didn't always work. Have the semantics changed so that this is guaranteed to work?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment75994>
kill the log.
return Failure("Failed to reap process(s) within timeout");
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment75996>
return Failure("Failed to reap process(s) within timeout");
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment75999>
can you include the error?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/20818/#comment76000>
How come we are not doing retires anymore?
- Vinod Kone
On May 5, 2014, 8:16 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 8:16 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
> src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
> src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
> src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
> src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/#review42251
-----------------------------------------------------------
Bad patch!
Reviews applied: [20818]
Failed command: git apply --index 20818.patch
Error:
error: patch failed: src/linux/cgroups.cpp:1510
error: src/linux/cgroups.cpp: patch does not apply
error: patch failed: src/tests/cgroups_tests.cpp:791
error: src/tests/cgroups_tests.cpp: patch does not apply
- Mesos ReviewBot
On May 5, 2014, 8:16 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20818/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 8:16 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-759 and MESOS-976
> https://issues.apache.org/jira/browse/MESOS-759
> https://issues.apache.org/jira/browse/MESOS-976
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
> to kill all processes in a freezer cgroup. cgroups::destroy will not
> complete until all processes have been reaped. A timeout is used for
> each step so destroy will eventually fail rather than constantly
> retrying.
>
> Added a test for destroying a freezer cgroup containing a stopped
> process.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
> src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
> src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
> src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
> src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
>
> Diff: https://reviews.apache.org/r/20818/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20818: Refactored cgroups::destroy to use a single pass
and to reap processes.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20818/
-----------------------------------------------------------
(Updated May 5, 2014, 1:16 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Jie Yu, and Vinod Kone.
Changes
-------
Added bugs.
Bugs: MESOS-759 and MESOS-976
https://issues.apache.org/jira/browse/MESOS-759
https://issues.apache.org/jira/browse/MESOS-976
Repository: mesos-git
Description
-------
internal::Destroyer now uses a single pass of freeze, kill, thaw, reap
to kill all processes in a freezer cgroup. cgroups::destroy will not
complete until all processes have been reaped. A timeout is used for
each step so destroy will eventually fail rather than constantly
retrying.
Added a test for destroying a freezer cgroup containing a stopped
process.
Diffs
-----
src/linux/cgroups.hpp 5a5735721fb9f051eee661edb08d1cdaa163d0f3
src/linux/cgroups.cpp 8202c282f580d027a60ded2081962e96e4860f60
src/slave/containerizer/isolators/cgroups/cpushare.cpp b494a9236210245383e20fa9ab3dbac01e42f8dd
src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
src/tests/cgroups_tests.cpp 6ba9de622953e158feadaa9950618b0b13c9e832
Diff: https://reviews.apache.org/r/20818/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes