You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Brenden Matthews <br...@diddyinc.com> on 2013/05/14 02:21:10 UTC

Review Request: For cgroups killTasks(): kill, freeze, kill, thaw.

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

Review request for mesos.


Description
-------

>From 7265fc2324a9a78a9d63026fef81c04effcd1bc0 Mon Sep 17 00:00:00 2001
From: Brenden Matthews <br...@airbnb.com>
Date: Fri, 10 May 2013 16:00:15 -0700
Subject: [PATCH 24/24] For cgroups killTasks(): kill, freeze, kill, thaw.

Review: https://reviews.apache.org/r/11131
---
 src/linux/cgroups.cpp |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


Diffs
-----

  src/linux/cgroups.cpp cfdc3b2d9203920021fdd891265d3595baa670ff 

Diff: https://reviews.apache.org/r/11131/diff/


Testing
-------

Used in production at airbnb.


Thanks,

Brenden Matthews


Re: Review Request: For cgroups killTasks(): kill, freeze, kill, thaw.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/#review21885
-----------------------------------------------------------



src/linux/cgroups.cpp
<https://reviews.apache.org/r/11131/#comment45178>

    Can you update this to do the following sequence?
    
    stop -> kill -> continue -> empty -> freeze -> kill -> thaw -> empty
    
    where stop/continue send SIGSTOP/SIGCONT
    
    You'll want to split up the two sequences:
    1. stop -> kill -> continue -> empty
    2. freeze -> kill -> thaw -> empty
    
    kill can take an 'int signal' so that you don't have to create additional functions for stop/continue.
    
    The second chain should only be executed when the first chain does not succeed. The easiest way to do this might be to create a "killed" function that serves as an intermediate version of "finished". Does this all make sense?


- Ben Mahler


On June 11, 2013, 9:45 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated June 11, 2013, 9:45 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> For cgroups killTasks(): kill, freeze, kill, thaw.
> 
> Review: https://reviews.apache.org/r/11131
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11131: Changed cgroups killTasks() sequence.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/#review23732
-----------------------------------------------------------

Ship it!


Modulo comments below.


src/linux/cgroups.cpp
<https://reviews.apache.org/r/11131/#comment47608>

    s/kill/signal/



src/linux/cgroups.cpp
<https://reviews.apache.org/r/11131/#comment47609>

    3 parts? How about:
    
    // The sequence of operations to kill a cgroup is as follows:
    // SIGSTOP -> SIGKILL -> empty -> freeze -> SIGKILL -> thaw -> empty
    // This process is repeated until the cgroup becomes empty.


- Ben Mahler


On July 23, 2013, 9:47 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated July 23, 2013, 9:47 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 5207ded352b0f6ce7922d59d6b47f2dd1a71538d 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11131: Changed cgroups killTasks() sequence.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/
-----------------------------------------------------------

(Updated July 24, 2013, 10:03 p.m.)


Review request for mesos.


Changes
-------

Updated as per Ben's comments.


Repository: mesos


Description
-------

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


Diffs (updated)
-----

  src/linux/cgroups.cpp 5207ded352b0f6ce7922d59d6b47f2dd1a71538d 

Diff: https://reviews.apache.org/r/11131/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request 11131: Changed cgroups killTasks() sequence.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/
-----------------------------------------------------------

(Updated July 23, 2013, 9:47 p.m.)


Review request for mesos.


Changes
-------

Updated as per Ben's comments.


Repository: mesos


Description
-------

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


Diffs (updated)
-----

  src/linux/cgroups.cpp 5207ded352b0f6ce7922d59d6b47f2dd1a71538d 

Diff: https://reviews.apache.org/r/11131/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request 11131: Changed cgroups killTasks() sequence.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/#review23670
-----------------------------------------------------------


This looks good! Just some small cleanup below.


src/linux/cgroups.cpp
<https://reviews.apache.org/r/11131/#comment47566>

    I think we can kill the VLOG, unless you think it will be useful? These processes could go away during normal operation so it doesn't seem like a failure.



src/linux/cgroups.cpp
<https://reviews.apache.org/r/11131/#comment47567>

    Update the comment?



src/linux/cgroups.cpp
<https://reviews.apache.org/r/11131/#comment47568>

    LGTM! Hopefully the freezer becomes more stable over time, and we can use it reliably =/


- Ben Mahler


On June 17, 2013, 7:36 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated June 17, 2013, 7:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/
-----------------------------------------------------------

(Updated June 17, 2013, 7:36 p.m.)


Review request for mesos.


Changes
-------

I typo'd the last patch.


Description (updated)
-------

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


Diffs (updated)
-----

  src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 

Diff: https://reviews.apache.org/r/11131/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/
-----------------------------------------------------------

(Updated June 17, 2013, 7:22 p.m.)


Review request for mesos.


Changes
-------

Ignore ESRCH errors from kill() in cgroups::kill().


Description (updated)
-------

Changed cgroups killTasks() sequence.

The sequence is as follows:
  stop -> kill -> empty -> freeze -> kill -> thaw -> empty

We also now ignore ERSCH errors from kill() in cgroups::kill().

Review: https://reviews.apache.org/r/11131


Diffs (updated)
-----

  src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 

Diff: https://reviews.apache.org/r/11131/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Vinod Kone <vi...@gmail.com>.

> On June 14, 2013, 2:24 a.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, lines 1515-1518
> > <https://reviews.apache.org/r/11131/diff/5/?file=304997#file304997line1515>
> >
> >     On second thought, let's just keep it simple for now as I imagine it's alright to freeze an empty cgroup.
> >     
> >     So stop -> kill -> empty -> freeze -> kill -> thaw -> empty. Then we don't need to be concerned with only continuing the chain when the kill doesn't work.
> >     
> >     Disregard SIGCONT, I was mislead by some comments in killtree.sh but it's not necessary when using SIGKILL.
> >     
> >     I apologize for the conflicting feedback!! To be clear, we haven't made any progress either in terms of figuring out what the problem is so we're agreed on this fix! :)

can't we just do stop->kill->freeze->kill->thaw->empty?


- Vinod


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


On June 14, 2013, 1:14 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated June 14, 2013, 1:14 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> Changed cgroups killTasks() sequence.
> 
> The sequence was broken into 2 steps:
>   1. stop, kill, continue; then
>   2. freeze, kill, thaw (if not empty)
> 
> Review: https://reviews.apache.org/r/11131
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Ben Mahler <be...@gmail.com>.

> On June 14, 2013, 2:24 a.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, lines 1515-1518
> > <https://reviews.apache.org/r/11131/diff/5/?file=304997#file304997line1515>
> >
> >     On second thought, let's just keep it simple for now as I imagine it's alright to freeze an empty cgroup.
> >     
> >     So stop -> kill -> empty -> freeze -> kill -> thaw -> empty. Then we don't need to be concerned with only continuing the chain when the kill doesn't work.
> >     
> >     Disregard SIGCONT, I was mislead by some comments in killtree.sh but it's not necessary when using SIGKILL.
> >     
> >     I apologize for the conflicting feedback!! To be clear, we haven't made any progress either in terms of figuring out what the problem is so we're agreed on this fix! :)
> 
> Vinod Kone wrote:
>     can't we just do stop->kill->freeze->kill->thaw->empty?
> 
> Brenden Matthews wrote:
>     Ben? Thoughts on this?

We can, but it doesn't allow any time for the killed processes to go away prior to freezing, I'd prefer empty() after the kill().


- Ben


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


On June 17, 2013, 7:36 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated June 17, 2013, 7:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On June 14, 2013, 2:24 a.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, lines 1515-1518
> > <https://reviews.apache.org/r/11131/diff/5/?file=304997#file304997line1515>
> >
> >     On second thought, let's just keep it simple for now as I imagine it's alright to freeze an empty cgroup.
> >     
> >     So stop -> kill -> empty -> freeze -> kill -> thaw -> empty. Then we don't need to be concerned with only continuing the chain when the kill doesn't work.
> >     
> >     Disregard SIGCONT, I was mislead by some comments in killtree.sh but it's not necessary when using SIGKILL.
> >     
> >     I apologize for the conflicting feedback!! To be clear, we haven't made any progress either in terms of figuring out what the problem is so we're agreed on this fix! :)
> 
> Vinod Kone wrote:
>     can't we just do stop->kill->freeze->kill->thaw->empty?

Ben? Thoughts on this?


- Brenden


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


On June 17, 2013, 7:22 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated June 17, 2013, 7:22 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> Changed cgroups killTasks() sequence.
> 
> The sequence is as follows:
>   stop -> kill -> empty -> freeze -> kill -> thaw -> empty
> 
> We also now ignore ERSCH errors from kill() in cgroups::kill().
> 
> Review: https://reviews.apache.org/r/11131
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On June 14, 2013, 2:24 a.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, lines 1515-1518
> > <https://reviews.apache.org/r/11131/diff/5/?file=304997#file304997line1515>
> >
> >     On second thought, let's just keep it simple for now as I imagine it's alright to freeze an empty cgroup.
> >     
> >     So stop -> kill -> empty -> freeze -> kill -> thaw -> empty. Then we don't need to be concerned with only continuing the chain when the kill doesn't work.
> >     
> >     Disregard SIGCONT, I was mislead by some comments in killtree.sh but it's not necessary when using SIGKILL.
> >     
> >     I apologize for the conflicting feedback!! To be clear, we haven't made any progress either in terms of figuring out what the problem is so we're agreed on this fix! :)
> 
> Vinod Kone wrote:
>     can't we just do stop->kill->freeze->kill->thaw->empty?
> 
> Brenden Matthews wrote:
>     Ben? Thoughts on this?
> 
> Ben Mahler wrote:
>     We can, but it doesn't allow any time for the killed processes to go away prior to freezing, I'd prefer empty() after the kill().

That's what I thought too.


- Brenden


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


On June 17, 2013, 7:36 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated June 17, 2013, 7:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/#review21893
-----------------------------------------------------------



src/linux/cgroups.cpp
<https://reviews.apache.org/r/11131/#comment45187>

    On second thought, let's just keep it simple for now as I imagine it's alright to freeze an empty cgroup.
    
    So stop -> kill -> empty -> freeze -> kill -> thaw -> empty. Then we don't need to be concerned with only continuing the chain when the kill doesn't work.
    
    Disregard SIGCONT, I was mislead by some comments in killtree.sh but it's not necessary when using SIGKILL.
    
    I apologize for the conflicting feedback!! To be clear, we haven't made any progress either in terms of figuring out what the problem is so we're agreed on this fix! :)


- Ben Mahler


On June 14, 2013, 1:14 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated June 14, 2013, 1:14 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> Changed cgroups killTasks() sequence.
> 
> The sequence was broken into 2 steps:
>   1. stop, kill, continue; then
>   2. freeze, kill, thaw (if not empty)
> 
> Review: https://reviews.apache.org/r/11131
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/#review21909
-----------------------------------------------------------


Can you add some code to ignore ESRCH in cgroups::kill? This ensures cgroups::kill doesn't return an Error when processes go away.

- Ben Mahler


On June 14, 2013, 1:14 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated June 14, 2013, 1:14 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> Changed cgroups killTasks() sequence.
> 
> The sequence was broken into 2 steps:
>   1. stop, kill, continue; then
>   2. freeze, kill, thaw (if not empty)
> 
> Review: https://reviews.apache.org/r/11131
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request: Changed cgroups killTasks() sequence.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/
-----------------------------------------------------------

(Updated June 14, 2013, 1:14 a.m.)


Review request for mesos.


Changes
-------

Updated as per Ben's comments.

make check'd.


Summary (updated)
-----------------

Changed cgroups killTasks() sequence.


Description (updated)
-------

Changed cgroups killTasks() sequence.

The sequence was broken into 2 steps:
  1. stop, kill, continue; then
  2. freeze, kill, thaw (if not empty)

Review: https://reviews.apache.org/r/11131


Diffs (updated)
-----

  src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 

Diff: https://reviews.apache.org/r/11131/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request: For cgroups killTasks(): kill, freeze, kill, thaw.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/
-----------------------------------------------------------

(Updated June 11, 2013, 9:45 p.m.)


Review request for mesos.


Changes
-------

Rebasing on master.


Description
-------

For cgroups killTasks(): kill, freeze, kill, thaw.

Review: https://reviews.apache.org/r/11131


Diffs (updated)
-----

  src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 

Diff: https://reviews.apache.org/r/11131/diff/


Testing (updated)
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request: For cgroups killTasks(): kill, freeze, kill, thaw.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/
-----------------------------------------------------------

(Updated June 6, 2013, 2:13 a.m.)


Review request for mesos.


Changes
-------

Rebasing on master.


Description
-------

For cgroups killTasks(): kill, freeze, kill, thaw.

Review: https://reviews.apache.org/r/11131


Diffs (updated)
-----

  src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 

Diff: https://reviews.apache.org/r/11131/diff/


Testing
-------

Used in production at airbnb.


Thanks,

Brenden Matthews


Re: Review Request: For cgroups killTasks(): kill, freeze, kill, thaw.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/
-----------------------------------------------------------

(Updated June 3, 2013, 5:57 p.m.)


Review request for mesos.


Description (updated)
-------

For cgroups killTasks(): kill, freeze, kill, thaw.

Review: https://reviews.apache.org/r/11131


Diffs (updated)
-----

  src/linux/cgroups.cpp 8d94fe63610c4c7a48f92d260fcd526b7a83942e 

Diff: https://reviews.apache.org/r/11131/diff/


Testing
-------

Used in production at airbnb.


Thanks,

Brenden Matthews


Re: Review Request: For cgroups killTasks(): kill, freeze, kill, thaw.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11131/#review20726
-----------------------------------------------------------


FWIW, we have seen similar issues at Twitter recently. We are looking into understanding the root cause before we decide on a fix. So I'll skip reviewing this for now.

- Vinod Kone


On May 14, 2013, 12:21 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11131/
> -----------------------------------------------------------
> 
> (Updated May 14, 2013, 12:21 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> From 7265fc2324a9a78a9d63026fef81c04effcd1bc0 Mon Sep 17 00:00:00 2001
> From: Brenden Matthews <br...@airbnb.com>
> Date: Fri, 10 May 2013 16:00:15 -0700
> Subject: [PATCH 24/24] For cgroups killTasks(): kill, freeze, kill, thaw.
> 
> Review: https://reviews.apache.org/r/11131
> ---
>  src/linux/cgroups.cpp |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp cfdc3b2d9203920021fdd891265d3595baa670ff 
> 
> Diff: https://reviews.apache.org/r/11131/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>