> On Jan. 16, 2013, 11:07 p.m., Ben Mahler wrote: > > Looks like you forgot to add stout/proc.hpp?
gah. sorry. done > On Jan. 16, 2013, 11:07 p.m., Ben Mahler wrote: > > src/slave/reaper.hpp, line 47 > > <https://reviews.apache.org/r/8570/diff/3/?file=249260#file249260line47> > > > > Are you able to programmatically enforce that one of these holds? not easily, no. esp. 1 and 2, in posix compliant way. more importantly, what do you mean by "enforce"? what do you propose we do when one of those 3 conditions aren't satisfied? log a warning? > On Jan. 16, 2013, 11:07 p.m., Ben Mahler wrote: > > src/slave/reaper.hpp, line 51 > > <https://reviews.apache.org/r/8570/diff/3/?file=249260#file249260line51> > > > > I think this is more succinct without losing clarity: > > s/addProcess/monitor I like addProcess because of its similarity to addListener. But, I could be convinced otherwise. > On Jan. 16, 2013, 11:07 p.m., Ben Mahler wrote: > > src/slave/reaper.hpp, line 61 > > <https://reviews.apache.org/r/8570/diff/3/?file=249260#file249260line61> > > > > 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 good catch. done > On Jan. 16, 2013, 11:07 p.m., Ben Mahler wrote: > > src/slave/reaper.cpp, line 96 > > <https://reviews.apache.org/r/8570/diff/3/?file=249261#file249261line96> > > > > But what about condition 3 you listed, where we are a privileged user? Then we souldn't enter this if loop. > On Jan. 16, 2013, 11:07 p.m., Ben Mahler wrote: > > src/tests/reaper_tests.cpp, line 68 > > <https://reviews.apache.org/r/8570/diff/3/?file=249263#file249263line68> > > > > Don't use PLOG within a child! We may deadlock, right? fair enough. using EXIT instead. > On Jan. 16, 2013, 11:07 p.m., Ben Mahler wrote: > > src/tests/reaper_tests.cpp, line 76 > > <https://reviews.apache.org/r/8570/diff/3/?file=249263#file249263line76> > > > > kill newline I like to have a new line between 2 comment blocks if the first one is more general and applies to more than the 2nd block. makes sense? > On Jan. 16, 2013, 11:07 p.m., Ben Mahler wrote: > > src/tests/reaper_tests.cpp, line 80 > > <https://reviews.apache.org/r/8570/diff/3/?file=249263#file249263line80> > > > > ditto, let's not use any glog stuff in child processes using EXIT - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8570/#review15415 ----------------------------------------------------------- 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 > >
