> On Nov. 16, 2012, 7:09 p.m., Ben Mahler wrote: > > src/slave/process_based_isolation_module.cpp, line 291 > > <https://reviews.apache.org/r/8077/diff/2/?file=190545#file190545line291> > > > > Do you see a way to avoid introducing this boolean? > > Vinod Kone wrote: > Hmmm, not sure how. The thing is an executor process terminated signal > might still need a proper clean up (for eg: process group in process based > isolation module or cgroup in cgroup based isolation module). Also, note that > killExecutor() can be called by the slave. Fwiw, the boolean 'killed' here is > used similarly in cgroups isolation module.
Ah, yes, I had a bit of deja-vu when looking at this ;) To me it seems like killExecutor() should be smart enough to be called several times, so that we can avoid having to introduce more state (i.e. the killed boolean). I could be convinced otherwise though, since I don't know this code very well. I guess one advantage of ensuring it's only called once is that we can LOG any redundant/invalid kill attempts? - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8077/#review13523 ----------------------------------------------------------- On Nov. 15, 2012, 11:54 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8077/ > ----------------------------------------------------------- > > (Updated Nov. 15, 2012, 11:54 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > This paves the way for a slave to shutdown executors that are not responsive > during launch (e.g. HDFS hang during fetching executor). > > This fix is not enough for cgroups isolation module (enough for the process > based isolation module) because it currently blocks while launching an > executor. > > > This addresses bug MESOS-310. > https://issues.apache.org/jira/browse/MESOS-310 > > > Diffs > ----- > > src/master/hierarchical_allocator_process.hpp > 5bcbeb99c823c3dbca0f89e19473b943c7469dab > src/master/master.cpp 87fad0ed1eaecd59f2814d3e776b746b754b79b7 > src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c > src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 > src/slave/process_based_isolation_module.hpp > efe59ebc0e8120926ea9f36b9eaa2f0b25830faf > src/slave/process_based_isolation_module.cpp > 16fd584e78db2c517d828f2576ab8a38c5ce57ad > src/slave/reaper.cpp c0ee4b4c07fd792bcb39455b666808b712eb32c2 > src/slave/slave.hpp bbba4404e9e2b1ff1e246f017cdad704438973ba > src/slave/slave.cpp 7deb4574943aae4cfc5da5d6b3f600042686975f > src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed > > Diff: https://reviews.apache.org/r/8077/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
