> On March 6, 2014, 7:35 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp, line 51
> > <https://reviews.apache.org/r/18595/diff/1/?file=506546#file506546line51>
> >
> >     This seems a little strange, because we'll be trying to signal multiple 
> > times:
> >     
> >              root <- signaled 1 time
> >             child <- signaled 2 times
> >        grandchild <- signaled 3 times
> >               ...
> >     Nth level pid <- signaled N times
> >     
> >     Also, how will this be used? Let's say you use this for a 5 minute 
> > signal escalation on the tree originally returned by killtree(pid_t, int, 
> > bool, bool). 5 minutes later, how do you know the pids are still valid and 
> > you don't accidentally kill unrelated processes?
> 
> Ben Mahler wrote:
>     Actually it's probably more productive to review r/18597 first. :)
> 
> Niklas Nielsen wrote:
>     This RR was mostly to address killing orphan processes or in other words, 
> have a way to kill the processes that belong to a process tree root which 
> might shutdown before its children. With the current killtree implementation, 
> it seems to me that this would fail (the pid can't be resolved to a process 
> and returns). Till mentioned that we might end up killing new processes same 
> pids, which is a problem.
>     
>     I am not sure how we can address both; I'll take a second stab at it 
> tomorrow :)
> 
> Till Toenshoff wrote:
>     Yep, I am a bit concerned about pid-reuse. As all our options are 
> poll-based (this approach in connection with RR18594 and a 
> delayed-continuation) as well as process::reaper, this approach seemed a bit 
> "safer" in regard to the polling period. process::reaper uses 1 second 
> whereas the use of this can be freely adjusted to smaller intervals (see 
> implementation of PluggableContainerizer's destroy escalation). In the end 
> however, all solutions we have so far appear to carry the risk of killing 
> unrelated (reused) pids. 
>     
>     For the signal repetition on childs within the same pid session or group, 
> that indeed might be a great point and me might want to address it.
> 
> Till Toenshoff wrote:
>     Just found some well-founded information on pid-reuse on linux: 
> http://stackoverflow.com/questions/11323410/linux-pid-recycling
>

Thanks for the pointer on pid-reuse!
I think a general issue is whether it is safe (or whether we should) interfere 
with further shutdown of the process root's children.
AFAIK We can't safely reap them as processes deeper in the tree might reap 
and/or ignore SIGCHLD?

Another way would be to signal the process groups after the delay (if the root 
is no longer around). However, group id reuse will be a similar problem.

It would significantly simplify escalation, if we sent SIGKILL to the process 
root (and its children) after a delay (potentially leaving behind orphan 
processes if the root exited before children did).


- Niklas


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


On Feb. 27, 2014, 4:54 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18595/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2014, 4:54 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> New killtree(ProcessTree tree, int signal) traverse process tree and sends a 
> signal to all pids. This is done regardless of presence and state of process.
> Patch is used by up coming signal escalation.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp 1f45897 
> 
> Diff: https://reviews.apache.org/r/18595/diff/
> 
> 
> Testing
> -------
> 
> Functional testing with signal escalation code and make check.
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Reply via email to