----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8570/#review15415 -----------------------------------------------------------
Looks like you forgot to add stout/proc.hpp? src/linux/proc.cpp <https://reviews.apache.org/r/8570/#comment33250> newline src/linux/proc.cpp <https://reviews.apache.org/r/8570/#comment33251> shouldn't this be ordered before sys? src/slave/reaper.hpp <https://reviews.apache.org/r/8570/#comment33259> Not yours, but seems overly verbose: s/addProcessExitedListener/addListener src/slave/reaper.hpp <https://reviews.apache.org/r/8570/#comment33256> Are you able to programmatically enforce that one of these holds? src/slave/reaper.hpp <https://reviews.apache.org/r/8570/#comment33257> I think this is more succinct without losing clarity: s/addProcess/monitor src/slave/reaper.hpp <https://reviews.apache.org/r/8570/#comment33260> Why do we use a set for the listeners? Can you please change this to a list? std::set is ordered, and our listeners do not provide comparison operations src/slave/reaper.cpp <https://reviews.apache.org/r/8570/#comment33253> ditto on ordered before sys? src/slave/reaper.cpp <https://reviews.apache.org/r/8570/#comment33264> Use "Failed ..." format? src/slave/reaper.cpp <https://reviews.apache.org/r/8570/#comment33265> But what about condition 3 you listed, where we are a privileged user? src/tests/reaper_tests.cpp <https://reviews.apache.org/r/8570/#comment33267> bad indentation src/tests/reaper_tests.cpp <https://reviews.apache.org/r/8570/#comment33273> Don't use PLOG within a child! We may deadlock, right? src/tests/reaper_tests.cpp <https://reviews.apache.org/r/8570/#comment33269> formatting src/tests/reaper_tests.cpp <https://reviews.apache.org/r/8570/#comment33270> kill newline src/tests/reaper_tests.cpp <https://reviews.apache.org/r/8570/#comment33274> ditto, let's not use any glog stuff in child processes - Ben Mahler On Jan. 16, 2013, 9:54 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8570/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2013, 9:54 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > Needed this to properly monitor the exit status of re-connected executors, as > they will be parented by INIT. > > > Diffs > ----- > > src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 > src/linux/proc.hpp 6c84a73ab1c4d6893390f96eae15422c21001328 > src/linux/proc.cpp 99df77447ddfb07a5febec9ebfed396778665041 > src/slave/reaper.hpp b9aa62daa42bdaa736ade43884982529ba3d4bb1 > src/slave/reaper.cpp c0ee4b4c07fd792bcb39455b666808b712eb32c2 > src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed > src/tests/reaper_tests.cpp PRE-CREATION > src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 > third_party/libprocess/Makefile.am 6b68c69b07b831255d34a68d7b9b4b37eee55167 > > Diff: https://reviews.apache.org/r/8570/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
