> On Jan. 22, 2013, 4:21 a.m., Benjamin Hindman wrote: > > src/tests/reaper_tests.cpp, lines 45-47 > > <https://reviews.apache.org/r/8570/diff/4/?file=250446#file250446line45> > > > > In other tests, we used 'ASSERT_NE(-1, pipe(pipes));'. While I like the > > idea of getting more debug info from the PLOG, I also like the idea of only > > aborting this test case. Does a PASSERT exist?
agreed on aborting only this test case. couldn't find anything akin to PASSERT. > On Jan. 22, 2013, 4:21 a.m., Benjamin Hindman wrote: > > src/tests/reaper_tests.cpp, line 54 > > <https://reviews.apache.org/r/8570/diff/4/?file=250446#file250446line54> > > > > I'm guilty of this too, but we really should not treat 'pid' as a > > boolean, because it is not. We should probably do a general "sweep" through > > the code and rectify everywhere. done. here and everywhere else. > On Jan. 22, 2013, 4:21 a.m., Benjamin Hindman wrote: > > src/tests/reaper_tests.cpp, line 70 > > <https://reviews.apache.org/r/8570/diff/4/?file=250446#file250446line70> > > > > This is a better place for doing something like PLOG, but you'll want > > to use perror since a child process might have the dreaded glog fork > > deadlock. Note that you _DO_ want to kill the entire process in this case > > otherwise you'll have two processes executing test cases, which gets very > > confusing for the person running tests. agreed. fixed > On Jan. 22, 2013, 4:21 a.m., Benjamin Hindman wrote: > > src/tests/reaper_tests.cpp, lines 79-80 > > <https://reviews.apache.org/r/8570/diff/4/?file=250446#file250446line79> > > > > If you want to be sure, use a pipe. I decided to be lazy and use a while loop instead. > On Jan. 22, 2013, 4:21 a.m., Benjamin Hindman wrote: > > src/slave/reaper.cpp, line 47 > > <https://reviews.apache.org/r/8570/diff/4/?file=250444#file250444line47> > > > > Cool. I like the cleanup, but I still think the long term solution here > > is to return a Future that gets satisfied once a pid is gone (reaped by us > > or otherwise). as discussed offline, I'm going to make this a TODO for now. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8570/#review15556 ----------------------------------------------------------- On Jan. 19, 2013, 1:52 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8570/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2013, 1:52 a.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 0ab59b75b2955c532d0833f132bdaffe323838d0 > src/linux/proc.hpp e0825a4a9f9e2763e0c25d7319f220bfe7c7c29c > src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4 > src/slave/cgroups_isolation_module.cpp > 63cefc33cf34eebb82db5d8448b751be8652fa36 > src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c > src/slave/process_based_isolation_module.cpp > 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 > 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 11f8b36d19313f65f1b191a781c6d5ed90130099 > third_party/libprocess/include/stout/proc.hpp PRE-CREATION > > Diff: https://reviews.apache.org/r/8570/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
