> On Feb. 15, 2013, 2:02 a.m., Ben Mahler wrote: > > src/slave/reaper.cpp, line 103 > > <https://reviews.apache.org/r/8570/diff/6/?file=258674#file258674line103> > > > > You're passing -1 to the listeners here! Can you propagate an error > > instead when the call fails? > > > > Right now you log the warning, but then notify anyway. This should > > either not notify them, or notify them of the error, I lack the context to > > know which is more appropriate atm.
We definitely should notify them, because the process exited. We send -1 status, when we can't determine status. The slave appropriately logs "unknown signal" when it gets this executorTerminated(). > On Feb. 15, 2013, 2:02 a.m., Ben Mahler wrote: > > third_party/libprocess/include/stout/proc.hpp, line 33 > > <https://reviews.apache.org/r/8570/diff/6/?file=258684#file258684line33> > > > > What else do you see going in here? > > > > I'd imagine linux/proc.hpp moving to stout, at which point do you > > expect to consolidate the two? If not, then we may want to move this > > function into os.hpp. I can imagine things like checking if a process is orphaned, sending signals (w/ appropriate error checking) etc to go here. Basically any helpers that make it easy to deal with processes. linux/proc.hpp is linux specific. So, when we move it into stout, I would imagine it be under linux directory in stout. I don't like to include it in os.hpp because 1) its already bloated and 2) proc would be a useful abstraction to deserve its own file. > On Feb. 15, 2013, 2:02 a.m., Ben Mahler wrote: > > src/tests/process_spawn.cpp, line 28 > > <https://reviews.apache.org/r/8570/diff/6/?file=258679#file258679line28> > > > > This file is outside our coding conventions, can you add a TODO to > > clean it up? cleaned up. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8570/#review16624 ----------------------------------------------------------- On Feb. 14, 2013, 3:30 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8570/ > ----------------------------------------------------------- > > (Updated Feb. 14, 2013, 3:30 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 c94736df660a25b58dc47c07d9c56c3c26152a66 > src/linux/proc.hpp e0825a4a9f9e2763e0c25d7319f220bfe7c7c29c > src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4 > src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d > src/slave/cgroups_isolation_module.cpp > 14f549edaf1b37a6bca8f75309864333ae775e7c > src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c > src/slave/process_based_isolation_module.cpp > 12a579cba56cd3dac384bc7919b0d5537b0e429d > src/slave/reaper.hpp b9aa62daa42bdaa736ade43884982529ba3d4bb1 > src/slave/reaper.cpp c0ee4b4c07fd792bcb39455b666808b712eb32c2 > src/slave/slave.cpp d4721c3eb51db87278d05f6fbe2eadb8a3a9b4dd > src/slave/solaris_project_isolation_module.cpp > f3b6a68926af34c46873d8de1c9858480f42ef98 > src/tests/cgroups_tests.cpp b219906374764e91f1a5268469ae92dd0fe08e53 > src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed > src/tests/process_spawn.cpp 04e836f8eed7312dbee27e20399e7d0e59df0bc2 > src/tests/reaper_tests.cpp PRE-CREATION > src/tests/script.cpp ebd2ab52e4de2dac744712b5adb1107a33ed29df > src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 > third_party/libprocess/Makefile.am dad1b65c3fdb7dbdad4e7c3d9c241cc4e89c3325 > third_party/libprocess/include/stout/proc.hpp PRE-CREATION > > Diff: https://reviews.apache.org/r/8570/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
