----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65551/#review197298 -----------------------------------------------------------
Mostly LGTM... some suggestions: src/launcher/default_executor.cpp Line 538 (original), 530 (patched) <https://reviews.apache.org/r/65551/#comment277429> Consider adding: `CHECK_EQ(containerIds.size(), reponses->size());` src/launcher/default_executor.cpp Line 1479 (original), 1492 (patched) <https://reviews.apache.org/r/65551/#comment277444> 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. - Joseph Wu On Feb. 9, 2018, 10:35 p.m., Gaston Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65551/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2018, 10:35 p.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 > >
