Mailing-List: contact mesos-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: mesos-dev@incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============0200548623552119809=="
MIME-Version: 1.0
Subject: Re: Review Request: Changed cgroups killTasks() sequence.
From: "Brenden Matthews" <brenden@diddyinc.com>
To: "Vinod Kone" <vinodkone@gmail.com>,
 "Ben Mahler" <benjamin.mahler@gmail.com>,
 "mesos" <mesos-dev@incubator.apache.org>,
 "Brenden Matthews" <brenden@diddyinc.com>
Date: Mon, 17 Jun 2013 19:47:10 -0000
Message-ID: <20130617194710.32192.45414@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Brenden Matthews" <noreply@reviews.apache.org>
References: <20130614022431.3558.93784@reviews.apache.org>
In-Reply-To: <20130614022431.3558.93784@reviews.apache.org>
Reply-To: "Brenden Matthews" <brenden@diddyinc.com>

--===============0200548623552119809==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


> 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=3D304997#file304997lin=
e1515>
> >
> >     On second thought, let's just keep it simple for now as I imagine i=
t's alright to freeze an empty cgroup.
> >     =

> >     So stop -> kill -> empty -> freeze -> kill -> thaw -> empty. Then w=
e don't need to be concerned with only continuing the chain when the kill d=
oesn't work.
> >     =

> >     Disregard SIGCONT, I was mislead by some comments in killtree.sh bu=
t 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 ha=
doop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> =

> =

> Thanks,
> =

> Brenden Matthews
> =

>


--===============0200548623552119809==--