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/06/18 20:53:09 UTC
Review Request: Add an API for destroying a cgroup (atomically kill all the
processes in it).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
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 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 Jie Yu <yu...@gmail.com>.
> On July 2, 2012, 5:18 a.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 615
> > <https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line615>
> >
> > Rather than keep this state, this is a great opportunity to fire off the 'killTasks' requests in parallel, then do a 'collect' on the futures!
Good idea! I will do it in the destroyer patch (the current patch only contains atomic kill stuff).
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/#review8781
-----------------------------------------------------------
On July 5, 2012, 10:07 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> 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.
>
>
> 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 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 for destroying a cgroup (atomically
kill all the processes in it).
Posted by Jie Yu <yu...@gmail.com>.
Yes. In that case, timeout is actually controlled by its caller.
- Jie
On Tue, Jul 3, 2012 at 8:53 AM, Benjamin Hindman <be...@berkeley.edu> wrote:
> As in, if the user discards the future, the destroy/freeze operation will
> be aborted?
>
> This sounds fine to me.
>
>
>
> On Mon, Jul 2, 2012 at 6:11 PM, Jie Yu <yu...@gmail.com> wrote:
>
>> Ben,
>>
>> I plan to remove the timeout parameter and let user decide when to cancel
>> the destroy (or freeze) operation.
>>
>> What do you think?
>>
>> - Jie
>>
>>
>> On Mon, Jul 2, 2012 at 5:34 PM, Jie Yu <yu...@gmail.com> wrote:
>>
>>> Ben,
>>>
>>> This patch depends on the freezer patch. Could you please review the
>>> freezer utils patch first? Thanks! (I will address the comments in this
>>> review anyway).
>>>
>>> https://reviews.apache.org/r/5401/
>>>
>>> - Jie
>>>
>>>
>>> On Sun, Jul 1, 2012 at 10:18 PM, Benjamin Hindman <be...@berkeley.edu>wrote:
>>>
>>>> This is an automatically generated e-mail. To reply, visit:
>>>> https://reviews.apache.org/r/5402/
>>>> src/linux/cgroups.hpp<https://reviews.apache.org/r/5402/diff/5/?file=115746#file115746line169> (Diff
>>>> revision 5)
>>>>
>>>> Try<process::Future<bool> > destroyCgroup(
>>>>
>>>> 169
>>>>
>>>> Option<double> timeout = Option<double>::none());
>>>>
>>>> const & for the Option<double>.
>>>>
>>>>
>>>> src/linux/cgroups.hpp<https://reviews.apache.org/r/5402/diff/5/?file=115746#file115746line273> (Diff
>>>> revision 5)
>>>>
>>>> Try<process::Future<bool> > killTasks(
>>>>
>>>> 273
>>>>
>>>> Option<double> timeout = Option<double>::none());
>>>>
>>>> Ditto.
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line601> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 601
>>>>
>>>> ~CgroupDestroyer() {}
>>>>
>>>> virtual.
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line603> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 603
>>>>
>>>> void initialize()
>>>>
>>>> This method is also virtual -- also, any reason not to keep this protected?
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line606> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 606
>>>>
>>>> state().onDiscarded(process::defer(this, &CgroupDestroyer::stop));
>>>>
>>>> Just like the 'EventListener', this should be a call to terminate.
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line613> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 613
>>>>
>>>> void start(Option<double> timeout)
>>>>
>>>> Why not pass timeout into the constructor and put all this logic in 'initialize'?
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line615> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 615
>>>>
>>>> if (index >= cgroups.size()){
>>>>
>>>> Rather than keep this state, this is a great opportunity to fire off the 'killTasks' requests in parallel, then do a 'collect' on the futures!
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line617> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 617
>>>>
>>>> process::dispatch(this, &CgroupDestroyer::stop);
>>>>
>>>> terminate
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line627> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 627
>>>>
>>>> process::dispatch(this, &CgroupDestroyer::stop);
>>>>
>>>> terminate
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line642> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 642
>>>>
>>>> Option<double> timeout)
>>>>
>>>> const &
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line648> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 648
>>>>
>>>> process::dispatch(this, &CgroupDestroyer::stop);
>>>>
>>>> terminate
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line655> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 655
>>>>
>>>> process::dispatch(this, &CgroupDestroyer::stop);
>>>>
>>>> terminate
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line659> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 659
>>>>
>>>> void stop()
>>>>
>>>> This function can be removed.
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line674> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 674
>>>>
>>>> Try<process::Future<bool> > destroyCgroup(const std::string& hierarchy,
>>>>
>>>> Just return the future, no need for the Try too.
>>>>
>>>>
>>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line705> (Diff
>>>> revision 5)
>>>>
>>>> public:
>>>>
>>>> 705
>>>>
>>>> process::dispatch(destroyer, &internal::CgroupDestroyer::start, timeout);
>>>>
>>>> See comment above, this extra dispatch can be removed.
>>>>
>>>>
>>>> - Benjamin
>>>>
>>>> On June 21st, 2012, 7:42 p.m., Jie Yu wrote:
>>>> Review request for mesos, Benjamin Hindman and Vinod Kone.
>>>> By Jie Yu.
>>>>
>>>> *Updated June 21, 2012, 7:42 p.m.*
>>>> 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.
>>>>
>>>> Testing
>>>>
>>>> On Linux machine, make check.
>>>>
>>>> Diffs
>>>>
>>>> - src/linux/cgroups.hpp (PRE-CREATION)
>>>> - src/linux/cgroups.cpp (PRE-CREATION)
>>>> - src/tests/cgroups_tests.cpp (PRE-CREATION)
>>>>
>>>> View Diff <https://reviews.apache.org/r/5402/diff/>
>>>>
>>>
>>>
>>
>
Re: Review Request: Add an API for destroying a cgroup (atomically
kill all the processes in it).
Posted by Benjamin Hindman <be...@berkeley.edu>.
As in, if the user discards the future, the destroy/freeze operation will
be aborted?
This sounds fine to me.
On Mon, Jul 2, 2012 at 6:11 PM, Jie Yu <yu...@gmail.com> wrote:
> Ben,
>
> I plan to remove the timeout parameter and let user decide when to cancel
> the destroy (or freeze) operation.
>
> What do you think?
>
> - Jie
>
>
> On Mon, Jul 2, 2012 at 5:34 PM, Jie Yu <yu...@gmail.com> wrote:
>
>> Ben,
>>
>> This patch depends on the freezer patch. Could you please review the
>> freezer utils patch first? Thanks! (I will address the comments in this
>> review anyway).
>>
>> https://reviews.apache.org/r/5401/
>>
>> - Jie
>>
>>
>> On Sun, Jul 1, 2012 at 10:18 PM, Benjamin Hindman <be...@berkeley.edu>wrote:
>>
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/5402/
>>> src/linux/cgroups.hpp<https://reviews.apache.org/r/5402/diff/5/?file=115746#file115746line169> (Diff
>>> revision 5)
>>>
>>> Try<process::Future<bool> > destroyCgroup(
>>>
>>> 169
>>>
>>> Option<double> timeout = Option<double>::none());
>>>
>>> const & for the Option<double>.
>>>
>>>
>>> src/linux/cgroups.hpp<https://reviews.apache.org/r/5402/diff/5/?file=115746#file115746line273> (Diff
>>> revision 5)
>>>
>>> Try<process::Future<bool> > killTasks(
>>>
>>> 273
>>>
>>> Option<double> timeout = Option<double>::none());
>>>
>>> Ditto.
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line601> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 601
>>>
>>> ~CgroupDestroyer() {}
>>>
>>> virtual.
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line603> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 603
>>>
>>> void initialize()
>>>
>>> This method is also virtual -- also, any reason not to keep this protected?
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line606> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 606
>>>
>>> state().onDiscarded(process::defer(this, &CgroupDestroyer::stop));
>>>
>>> Just like the 'EventListener', this should be a call to terminate.
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line613> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 613
>>>
>>> void start(Option<double> timeout)
>>>
>>> Why not pass timeout into the constructor and put all this logic in 'initialize'?
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line615> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 615
>>>
>>> if (index >= cgroups.size()){
>>>
>>> Rather than keep this state, this is a great opportunity to fire off the 'killTasks' requests in parallel, then do a 'collect' on the futures!
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line617> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 617
>>>
>>> process::dispatch(this, &CgroupDestroyer::stop);
>>>
>>> terminate
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line627> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 627
>>>
>>> process::dispatch(this, &CgroupDestroyer::stop);
>>>
>>> terminate
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line642> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 642
>>>
>>> Option<double> timeout)
>>>
>>> const &
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line648> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 648
>>>
>>> process::dispatch(this, &CgroupDestroyer::stop);
>>>
>>> terminate
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line655> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 655
>>>
>>> process::dispatch(this, &CgroupDestroyer::stop);
>>>
>>> terminate
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line659> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 659
>>>
>>> void stop()
>>>
>>> This function can be removed.
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line674> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 674
>>>
>>> Try<process::Future<bool> > destroyCgroup(const std::string& hierarchy,
>>>
>>> Just return the future, no need for the Try too.
>>>
>>>
>>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line705> (Diff
>>> revision 5)
>>>
>>> public:
>>>
>>> 705
>>>
>>> process::dispatch(destroyer, &internal::CgroupDestroyer::start, timeout);
>>>
>>> See comment above, this extra dispatch can be removed.
>>>
>>>
>>> - Benjamin
>>>
>>> On June 21st, 2012, 7:42 p.m., Jie Yu wrote:
>>> Review request for mesos, Benjamin Hindman and Vinod Kone.
>>> By Jie Yu.
>>>
>>> *Updated June 21, 2012, 7:42 p.m.*
>>> 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.
>>>
>>> Testing
>>>
>>> On Linux machine, make check.
>>>
>>> Diffs
>>>
>>> - src/linux/cgroups.hpp (PRE-CREATION)
>>> - src/linux/cgroups.cpp (PRE-CREATION)
>>> - src/tests/cgroups_tests.cpp (PRE-CREATION)
>>>
>>> View Diff <https://reviews.apache.org/r/5402/diff/>
>>>
>>
>>
>
Re: Review Request: Add an API for destroying a cgroup (atomically
kill all the processes in it).
Posted by Jie Yu <yu...@gmail.com>.
Ben,
I plan to remove the timeout parameter and let user decide when to cancel
the destroy (or freeze) operation.
What do you think?
- Jie
On Mon, Jul 2, 2012 at 5:34 PM, Jie Yu <yu...@gmail.com> wrote:
> Ben,
>
> This patch depends on the freezer patch. Could you please review the
> freezer utils patch first? Thanks! (I will address the comments in this
> review anyway).
>
> https://reviews.apache.org/r/5401/
>
> - Jie
>
>
> On Sun, Jul 1, 2012 at 10:18 PM, Benjamin Hindman <be...@berkeley.edu>wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/5402/
>> src/linux/cgroups.hpp<https://reviews.apache.org/r/5402/diff/5/?file=115746#file115746line169> (Diff
>> revision 5)
>>
>> Try<process::Future<bool> > destroyCgroup(
>>
>> 169
>>
>> Option<double> timeout = Option<double>::none());
>>
>> const & for the Option<double>.
>>
>>
>> src/linux/cgroups.hpp<https://reviews.apache.org/r/5402/diff/5/?file=115746#file115746line273> (Diff
>> revision 5)
>>
>> Try<process::Future<bool> > killTasks(
>>
>> 273
>>
>> Option<double> timeout = Option<double>::none());
>>
>> Ditto.
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line601> (Diff
>> revision 5)
>>
>> public:
>>
>> 601
>>
>> ~CgroupDestroyer() {}
>>
>> virtual.
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line603> (Diff
>> revision 5)
>>
>> public:
>>
>> 603
>>
>> void initialize()
>>
>> This method is also virtual -- also, any reason not to keep this protected?
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line606> (Diff
>> revision 5)
>>
>> public:
>>
>> 606
>>
>> state().onDiscarded(process::defer(this, &CgroupDestroyer::stop));
>>
>> Just like the 'EventListener', this should be a call to terminate.
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line613> (Diff
>> revision 5)
>>
>> public:
>>
>> 613
>>
>> void start(Option<double> timeout)
>>
>> Why not pass timeout into the constructor and put all this logic in 'initialize'?
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line615> (Diff
>> revision 5)
>>
>> public:
>>
>> 615
>>
>> if (index >= cgroups.size()){
>>
>> Rather than keep this state, this is a great opportunity to fire off the 'killTasks' requests in parallel, then do a 'collect' on the futures!
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line617> (Diff
>> revision 5)
>>
>> public:
>>
>> 617
>>
>> process::dispatch(this, &CgroupDestroyer::stop);
>>
>> terminate
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line627> (Diff
>> revision 5)
>>
>> public:
>>
>> 627
>>
>> process::dispatch(this, &CgroupDestroyer::stop);
>>
>> terminate
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line642> (Diff
>> revision 5)
>>
>> public:
>>
>> 642
>>
>> Option<double> timeout)
>>
>> const &
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line648> (Diff
>> revision 5)
>>
>> public:
>>
>> 648
>>
>> process::dispatch(this, &CgroupDestroyer::stop);
>>
>> terminate
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line655> (Diff
>> revision 5)
>>
>> public:
>>
>> 655
>>
>> process::dispatch(this, &CgroupDestroyer::stop);
>>
>> terminate
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line659> (Diff
>> revision 5)
>>
>> public:
>>
>> 659
>>
>> void stop()
>>
>> This function can be removed.
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line674> (Diff
>> revision 5)
>>
>> public:
>>
>> 674
>>
>> Try<process::Future<bool> > destroyCgroup(const std::string& hierarchy,
>>
>> Just return the future, no need for the Try too.
>>
>>
>> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line705> (Diff
>> revision 5)
>>
>> public:
>>
>> 705
>>
>> process::dispatch(destroyer, &internal::CgroupDestroyer::start, timeout);
>>
>> See comment above, this extra dispatch can be removed.
>>
>>
>> - Benjamin
>>
>> On June 21st, 2012, 7:42 p.m., Jie Yu wrote:
>> Review request for mesos, Benjamin Hindman and Vinod Kone.
>> By Jie Yu.
>>
>> *Updated June 21, 2012, 7:42 p.m.*
>> 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.
>>
>> Testing
>>
>> On Linux machine, make check.
>>
>> Diffs
>>
>> - src/linux/cgroups.hpp (PRE-CREATION)
>> - src/linux/cgroups.cpp (PRE-CREATION)
>> - src/tests/cgroups_tests.cpp (PRE-CREATION)
>>
>> View Diff <https://reviews.apache.org/r/5402/diff/>
>>
>
>
Re: Review Request: Add an API for destroying a cgroup (atomically
kill all the processes in it).
Posted by Jie Yu <yu...@gmail.com>.
Ben,
This patch depends on the freezer patch. Could you please review the
freezer utils patch first? Thanks! (I will address the comments in this
review anyway).
https://reviews.apache.org/r/5401/
- Jie
On Sun, Jul 1, 2012 at 10:18 PM, Benjamin Hindman <be...@berkeley.edu> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5402/
> src/linux/cgroups.hpp<https://reviews.apache.org/r/5402/diff/5/?file=115746#file115746line169> (Diff
> revision 5)
>
> Try<process::Future<bool> > destroyCgroup(
>
> 169
>
> Option<double> timeout = Option<double>::none());
>
> const & for the Option<double>.
>
>
> src/linux/cgroups.hpp<https://reviews.apache.org/r/5402/diff/5/?file=115746#file115746line273> (Diff
> revision 5)
>
> Try<process::Future<bool> > killTasks(
>
> 273
>
> Option<double> timeout = Option<double>::none());
>
> Ditto.
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line601> (Diff
> revision 5)
>
> public:
>
> 601
>
> ~CgroupDestroyer() {}
>
> virtual.
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line603> (Diff
> revision 5)
>
> public:
>
> 603
>
> void initialize()
>
> This method is also virtual -- also, any reason not to keep this protected?
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line606> (Diff
> revision 5)
>
> public:
>
> 606
>
> state().onDiscarded(process::defer(this, &CgroupDestroyer::stop));
>
> Just like the 'EventListener', this should be a call to terminate.
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line613> (Diff
> revision 5)
>
> public:
>
> 613
>
> void start(Option<double> timeout)
>
> Why not pass timeout into the constructor and put all this logic in 'initialize'?
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line615> (Diff
> revision 5)
>
> public:
>
> 615
>
> if (index >= cgroups.size()){
>
> Rather than keep this state, this is a great opportunity to fire off the 'killTasks' requests in parallel, then do a 'collect' on the futures!
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line617> (Diff
> revision 5)
>
> public:
>
> 617
>
> process::dispatch(this, &CgroupDestroyer::stop);
>
> terminate
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line627> (Diff
> revision 5)
>
> public:
>
> 627
>
> process::dispatch(this, &CgroupDestroyer::stop);
>
> terminate
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line642> (Diff
> revision 5)
>
> public:
>
> 642
>
> Option<double> timeout)
>
> const &
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line648> (Diff
> revision 5)
>
> public:
>
> 648
>
> process::dispatch(this, &CgroupDestroyer::stop);
>
> terminate
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line655> (Diff
> revision 5)
>
> public:
>
> 655
>
> process::dispatch(this, &CgroupDestroyer::stop);
>
> terminate
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line659> (Diff
> revision 5)
>
> public:
>
> 659
>
> void stop()
>
> This function can be removed.
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line674> (Diff
> revision 5)
>
> public:
>
> 674
>
> Try<process::Future<bool> > destroyCgroup(const std::string& hierarchy,
>
> Just return the future, no need for the Try too.
>
>
> src/linux/cgroups.cpp<https://reviews.apache.org/r/5402/diff/5/?file=115747#file115747line705> (Diff
> revision 5)
>
> public:
>
> 705
>
> process::dispatch(destroyer, &internal::CgroupDestroyer::start, timeout);
>
> See comment above, this extra dispatch can be removed.
>
>
> - Benjamin
>
> On June 21st, 2012, 7:42 p.m., Jie Yu wrote:
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> By Jie Yu.
>
> *Updated June 21, 2012, 7:42 p.m.*
> 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.
>
> Testing
>
> On Linux machine, make check.
>
> Diffs
>
> - src/linux/cgroups.hpp (PRE-CREATION)
> - src/linux/cgroups.cpp (PRE-CREATION)
> - src/tests/cgroups_tests.cpp (PRE-CREATION)
>
> View Diff <https://reviews.apache.org/r/5402/diff/>
>
Re: Review Request: Add an API for destroying a cgroup (atomically kill all
the processes in it).
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/#review8781
-----------------------------------------------------------
src/linux/cgroups.hpp
<https://reviews.apache.org/r/5402/#comment18586>
const & for the Option<double>.
src/linux/cgroups.hpp
<https://reviews.apache.org/r/5402/#comment18587>
Ditto.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18588>
virtual.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18589>
This method is also virtual -- also, any reason not to keep this protected?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18591>
Just like the 'EventListener', this should be a call to terminate.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18597>
Why not pass timeout into the constructor and put all this logic in 'initialize'?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18602>
Rather than keep this state, this is a great opportunity to fire off the 'killTasks' requests in parallel, then do a 'collect' on the futures!
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18592>
terminate
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18593>
terminate
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18601>
const &
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18594>
terminate
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18595>
terminate
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18596>
This function can be removed.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18599>
Just return the future, no need for the Try too.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5402/#comment18598>
See comment above, this extra dispatch can be removed.
- Benjamin Hindman
On June 21, 2012, 7:42 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5402/
> -----------------------------------------------------------
>
> (Updated June 21, 2012, 7:42 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 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
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 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 for destroying a cgroup (atomically kill all
the processes in it).
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated June 21, 2012, 7:42 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
A major refactor. Return a future for those functions and introduce timeout for each operation (instead of using await).
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 for destroying a cgroup (atomically kill all
the processes in it).
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated June 20, 2012, 4:50 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Adapt to the new cgroups basic APIs.
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 for destroying a cgroup (atomically kill all
the processes in it).
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated June 19, 2012, 5:59 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Add a timeout parameter to the destroy functions.
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 for destroying a cgroup (atomically kill all
the processes in it).
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5402/
-----------------------------------------------------------
(Updated June 19, 2012, 6:37 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
The newest version accomodate to the new gtest hack and the new freezer utils.
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