> 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
>
>
|