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



3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
<https://reviews.apache.org/r/11849/#comment45078>

    why the include here?



3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
<https://reviews.apache.org/r/11849/#comment45077>

    Are you missing the corresponding Makefile change?



3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
<https://reviews.apache.org/r/11849/#comment45082>

    I think it makes more sense to call 'stopped' set as 'visited' per typical 
graph algo terminology. I was a bit confused because i was equating stopped 
with processing getting a SIGSTOP.



3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
<https://reviews.apache.org/r/11849/#comment45083>

    You only add one process to 'stopped'/'visited' right in one iteration 
right?
    
    Why are you repeatedly sending SIGSTOP to the same process?
    
    Also, I highly recommend adding a verbose option to this function. It has 
proved invaluable to us when using/debugging the erstwhile killtree script.



3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
<https://reviews.apache.org/r/11849/#comment45084>

    ditto.



3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
<https://reviews.apache.org/r/11849/#comment45085>

    close(grandChildPipes[0]) ?



3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
<https://reviews.apache.org/r/11849/#comment45086>

    immediately after forking :)



3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
<https://reviews.apache.org/r/11849/#comment45087>

    s/discover/kill/ ? to be consistent with the next statement?



3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
<https://reviews.apache.org/r/11849/#comment45088>

    nice test!


- Vinod Kone


On June 12, 2013, 7:49 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11849/
> -----------------------------------------------------------
> 
> (Updated June 12, 2013, 7:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> For how killtree will be used, see the killtree.sh script in mesos.
> 
> This version is actually more robust than killtree.sh, as it first stops the 
> entire tree, _then_ issues the signal.
> Currently, killtree.sh stops, signals, and continues each process in 
> isolation.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 047778d05ebbbefd85e4a163dbb6ab8445edfb7f 
> 
> Diff: https://reviews.apache.org/r/11849/diff/
> 
> 
> Testing
> -------
> 
> Added a test, however this test can be _significantly_ cleaned up were we to 
> create an abstraction for generating process trees!
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to