> On Feb. 12, 2018, 7:57 p.m., Joseph Wu wrote: > > src/launcher/default_executor.cpp > > Line 1479 (original), 1492 (patched) > > <https://reviews.apache.org/r/65551/diff/4/?file=1955928#file1955928line1509> > > > > This booleans seems like a remnant of the time when the default > > executor would only launch a single task group. The value is initialized > > to false, and is flipped to true upon launching the first TaskGroup. > > > > It is `CHECK`'d in `__launchGroup(...)` (seems redundant, as that is a > > continuation of `launchGroup` where `launched = true;` is set); in `wait()` > > (similarly redundant, but has more entrypoints); and in `shutdown()`. > > > > You can consider removing the bool in a separate patch.
I made the same comment in the previous review :) - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65551/#review197298 ----------------------------------------------------------- On Feb. 10, 2018, 6:35 a.m., Gaston Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65551/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2018, 6:35 a.m.) > > > Review request for mesos, Anand Mazumdar, Greg Mann, Qian Zhang, and Vinod > Kone. > > > Bugs: MESOS-8468 > https://issues.apache.org/jira/browse/MESOS-8468 > > > Repository: mesos > > > Description > ------- > > The default executor would be completely shutdown on a > `LAUNCH_NESTED_CONTAINER` failure. > > This patch makes it kill the affected task group instead of shutting > down and killing all task groups. > > > Diffs > ----- > > src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea > > > Diff: https://reviews.apache.org/r/65551/diff/4/ > > > Testing > ------- > > `sudo make check` on GNU/Linux > > Regression test on https://reviews.apache.org/r/65552/ > > > Thanks, > > Gaston Kleiman > >
