----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51477/#review147401 -----------------------------------------------------------
src/slave/slave.cpp (line 1549) <https://reviews.apache.org/r/51477/#comment214588> I would suggest to change it to `ignoring running` in order to be consistent with the other warning message. src/slave/slave.cpp (lines 1686 - 1690) <https://reviews.apache.org/r/51477/#comment214556> Should it be `CHECK_NE(task.isSome(), taskGroup.isSome())`? src/slave/slave.cpp (line 1721) <https://reviews.apache.org/r/51477/#comment214589> s/run/running/ src/slave/slave.cpp (line 1752) <https://reviews.apache.org/r/51477/#comment214585> I think we can `break` from here. src/slave/slave.cpp (lines 1777 - 1778) <https://reviews.apache.org/r/51477/#comment214587> Kill one blank line. src/slave/slave.cpp (lines 1851 - 1852) <https://reviews.apache.org/r/51477/#comment214590> I think we need to `break` between these two `}` if kill == true. src/slave/slave.cpp (line 1879) <https://reviews.apache.org/r/51477/#comment214591> I do not think we need this, because if we get here, `kill` must be still `false`. src/slave/slave.cpp (line 1946) <https://reviews.apache.org/r/51477/#comment214592> Not yours, but I think we do not need this blank line. src/slave/slave.cpp (lines 2091 - 2095) <https://reviews.apache.org/r/51477/#comment214599> Should it be `CHECK_NE(tasks.isSome(), taskGroups.isSome())`? src/slave/slave.cpp (line 2521) <https://reviews.apache.org/r/51477/#comment214712> Here we send a `KillTaskMessage` to executor, but the task to be killed may be part of a task group, so that means it is executor's responsibility to kill the tasks in the whole group, right? What if executor does not do it? Will we enforce it in agent? - Qian Zhang On Aug. 28, 2016, 1:44 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51477/ > ----------------------------------------------------------- > > (Updated Aug. 28, 2016, 1:44 a.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-6076 > https://issues.apache.org/jira/browse/MESOS-6076 > > > Repository: mesos > > > Description > ------- > > This changes implements the `runTaskGroup()` handler on the > agent ensuring that task group is sent atomically to the executor > via the `LAUNCH_GROUP` event. It also refactors `runTask()`/`_runTask()` > to go through a common handler function. Also, it ensures that all > tasks in `framework->pending`/`queuedTasks` that are killed before > running the task group result in all the tasks being killed. > > > Diffs > ----- > > src/slave/slave.hpp 4add4c0180ea56039e0d5009bad4d9346128bde6 > src/slave/slave.cpp 5d162d00a8a9a0e318c39bda93dd584f23c87987 > > Diff: https://reviews.apache.org/r/51477/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Anand Mazumdar > >
