> On Feb. 15, 2013, 1:41 a.m., Ben Mahler wrote: > > src/slave/cgroups_isolation_module.cpp, line 437 > > <https://reviews.apache.org/r/9408/diff/6/?file=258837#file258837line437> > > > > Seems more like a LOG(ERROR) to me, given it's an unexpected error, and > > we can proceed to remove the rest of the cgroups, right?
I think this is a fatal enough error (un-cleaned cgroups => orphaned executors), that it needs manual intervention. > On Feb. 15, 2013, 1:41 a.m., Ben Mahler wrote: > > src/slave/cgroups_isolation_module.cpp, line 444 > > <https://reviews.apache.org/r/9408/diff/6/?file=258837#file258837line444> > > > > Ditto. ERROR or FATAL, given we expect to be able to remove the lock, > > right? This is not the case when writing googletest style tests. If a test can't properly unlock the file, the subsequent test would fail. > On Feb. 15, 2013, 1:41 a.m., Ben Mahler wrote: > > src/slave/process_based_isolation_module.cpp, line 94 > > <https://reviews.apache.org/r/9408/diff/6/?file=258839#file258839line94> > > > > Since killExecutor is called from finalize(), can you add a note there > > to indicate that no dispatches should be added to that method? > > Even with the note, this still feels a little error prone, should > > someone change killExecutor. Good point. Added a note. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9408/#review16622 ----------------------------------------------------------- On Feb. 14, 2013, 7:16 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9408/ > ----------------------------------------------------------- > > (Updated Feb. 14, 2013, 7:16 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > See summary > > > Diffs > ----- > > src/slave/cgroups_isolation_module.hpp > 669efa14ba2603764aa68ae19a44e79dbfdec192 > src/slave/cgroups_isolation_module.cpp > 14f549edaf1b37a6bca8f75309864333ae775e7c > src/slave/process_based_isolation_module.hpp > f1817192582e3646f8dcf17934ba7998829e8fd6 > src/slave/process_based_isolation_module.cpp > 12a579cba56cd3dac384bc7919b0d5537b0e429d > src/tests/balloon_framework_test.sh > 93a733f64cfde08349b7781eb3d5e13594c74498 > > Diff: https://reviews.apache.org/r/9408/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
