You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2012/07/06 00:07:02 UTC
Re: Review Request: Add an API to atomically kill all tasks in a cgroup.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated July 5, 2012, 10:07 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Refactor the code. Use .then to link several operations in TasksKill. Separate cgroup destroy code into a new patch (will submit later). Include a new test.
Summary (updated)
-----------------
Add an API to atomically kill all tasks in a cgroup.
Description
-------
This patch leverages the freezer subsystem in cgroups to kill all the processes in a cgroup atomically.
The main idea is to freeze all the processes in a cgroup first, then send kill signal to all the proceses. This avoids the need of walking the proc process tree to kill all processes associated with an executor. In fact, the original killtree solution assumes that the user processes haven't blocked the SIGSTOP signal, which may not be true in some cases.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5402/diff/
Testing
-------
On Linux machine, make check.
Thanks,
Jie Yu
Re: Review Request: Add an API to atomically kill all tasks in a cgroup.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/#review9616
-----------------------------------------------------------
Ship it!
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment20523>
Love this. Once the bug in libprocess is fixed (see comment below), then you can just start this with:
defer(self(), &Self::freeze)
.then(defer(self(), &Self::kill))
.then(...);
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment20525>
Ah ha, this is a bug in libprocess. If we discard an "outer" future, we need to make sure we clean up all the other futures as well. Please add a TODO here to remove this once libprocess is fixed (and don't hesitate to fix libprocess!).
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment20522>
I'd prefer you move this into a check at the beginning, ie.g., CHECK(!finish.isPending() || !finish.isDiscarded());
- Benjamin Hindman
On July 24, 2012, 10:47 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5402/
> -----------------------------------------------------------
>
> (Updated July 24, 2012, 10:47 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This patch leverages the freezer subsystem in cgroups to kill all the processes in a cgroup atomically.
>
> The main idea is to freeze all the processes in a cgroup first, then send kill signal to all the proceses. This avoids the need of walking the proc process tree to kill all processes associated with an executor. In fact, the original killtree solution assumes that the user processes haven't blocked the SIGSTOP signal, which may not be true in some cases.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp cec9e36
> src/linux/cgroups.cpp 93c140f
> src/tests/cgroups_tests.cpp 7f0f901
>
> Diff: https://reviews.apache.org/r/5402/diff/
>
>
> Testing
> -------
>
> On Linux machine, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add an API to atomically kill all tasks in a cgroup.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated July 31, 2012, 12:52 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Address Ben's comments.
Description
-------
This patch leverages the freezer subsystem in cgroups to kill all the processes in a cgroup atomically.
The main idea is to freeze all the processes in a cgroup first, then send kill signal to all the proceses. This avoids the need of walking the proc process tree to kill all processes associated with an executor. In fact, the original killtree solution assumes that the user processes haven't blocked the SIGSTOP signal, which may not be true in some cases.
Diffs (updated)
-----
src/linux/cgroups.hpp cec9e36
src/linux/cgroups.cpp 93c140f
src/tests/cgroups_tests.cpp 7f0f901
Diff: https://reviews.apache.org/r/5402/diff/
Testing
-------
On Linux machine, make check.
Thanks,
Jie Yu
Re: Review Request: Add an API to atomically kill all tasks in a cgroup.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated July 24, 2012, 10:47 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Addressed Ben's review comments. Make "interval" a well-typed seconds, and set default in prototype.
Description
-------
This patch leverages the freezer subsystem in cgroups to kill all the processes in a cgroup atomically.
The main idea is to freeze all the processes in a cgroup first, then send kill signal to all the proceses. This avoids the need of walking the proc process tree to kill all processes associated with an executor. In fact, the original killtree solution assumes that the user processes haven't blocked the SIGSTOP signal, which may not be true in some cases.
Diffs (updated)
-----
src/linux/cgroups.hpp cec9e36
src/linux/cgroups.cpp 93c140f
src/tests/cgroups_tests.cpp 7f0f901
Diff: https://reviews.apache.org/r/5402/diff/
Testing
-------
On Linux machine, make check.
Thanks,
Jie Yu
Re: Review Request: Add an API to atomically kill all tasks in a cgroup.
Posted by Jie Yu <yu...@gmail.com>.
> On July 24, 2012, 6:43 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.hpp, lines 305-306
> > <https://reviews.apache.org/r/5402/diff/8/?file=123393#file123393line305>
> >
> > Let's just thread the default from the beginning, and use a well typed duration such as 'seconds'.
Done.
> On July 24, 2012, 6:43 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 1147
> > <https://reviews.apache.org/r/5402/diff/8/?file=123394#file123394line1147>
> >
> > Kill newline.
Done.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/#review9402
-----------------------------------------------------------
On July 24, 2012, 10:47 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5402/
> -----------------------------------------------------------
>
> (Updated July 24, 2012, 10:47 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This patch leverages the freezer subsystem in cgroups to kill all the processes in a cgroup atomically.
>
> The main idea is to freeze all the processes in a cgroup first, then send kill signal to all the proceses. This avoids the need of walking the proc process tree to kill all processes associated with an executor. In fact, the original killtree solution assumes that the user processes haven't blocked the SIGSTOP signal, which may not be true in some cases.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp cec9e36
> src/linux/cgroups.cpp 93c140f
> src/tests/cgroups_tests.cpp 7f0f901
>
> Diff: https://reviews.apache.org/r/5402/diff/
>
>
> Testing
> -------
>
> On Linux machine, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add an API to atomically kill all tasks in a cgroup.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/#review9402
-----------------------------------------------------------
Update this review with the same comments as https://reviews.apache.org/r/5401 and then I'll do another pass!
src/linux/cgroups.hpp
<https://reviews.apache.org/r/5402/#comment20139>
Let's just thread the default from the beginning, and use a well typed duration such as 'seconds'.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment20140>
Kill newline.
- Benjamin Hindman
On July 16, 2012, 7:10 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5402/
> -----------------------------------------------------------
>
> (Updated July 16, 2012, 7:10 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This patch leverages the freezer subsystem in cgroups to kill all the processes in a cgroup atomically.
>
> The main idea is to freeze all the processes in a cgroup first, then send kill signal to all the proceses. This avoids the need of walking the proc process tree to kill all processes associated with an executor. In fact, the original killtree solution assumes that the user processes haven't blocked the SIGSTOP signal, which may not be true in some cases.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp cec9e36
> src/linux/cgroups.cpp 93c140f
> src/tests/cgroups_tests.cpp 7f0f901
>
> Diff: https://reviews.apache.org/r/5402/diff/
>
>
> Testing
> -------
>
> On Linux machine, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add an API to atomically kill all tasks in a cgroup.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated July 16, 2012, 7:10 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Updated to fit the new trunk.
Description
-------
This patch leverages the freezer subsystem in cgroups to kill all the processes in a cgroup atomically.
The main idea is to freeze all the processes in a cgroup first, then send kill signal to all the proceses. This avoids the need of walking the proc process tree to kill all processes associated with an executor. In fact, the original killtree solution assumes that the user processes haven't blocked the SIGSTOP signal, which may not be true in some cases.
Diffs (updated)
-----
src/linux/cgroups.hpp cec9e36
src/linux/cgroups.cpp 93c140f
src/tests/cgroups_tests.cpp 7f0f901
Diff: https://reviews.apache.org/r/5402/diff/
Testing
-------
On Linux machine, make check.
Thanks,
Jie Yu
Re: Review Request: Add an API to atomically kill all tasks in a cgroup.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated July 9, 2012, 6:50 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Fix format issues. Remove sub-cgroups check in killTasks (no needed).
Description
-------
This patch leverages the freezer subsystem in cgroups to kill all the processes in a cgroup atomically.
The main idea is to freeze all the processes in a cgroup first, then send kill signal to all the proceses. This avoids the need of walking the proc process tree to kill all processes associated with an executor. In fact, the original killtree solution assumes that the user processes haven't blocked the SIGSTOP signal, which may not be true in some cases.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5402/diff/
Testing
-------
On Linux machine, make check.
Thanks,
Jie Yu