----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/#review96139 -----------------------------------------------------------
Given the rather complicated setup of this ( destroy -> Destroyer -...-> TasksKiller/s ), I think we really need tests here and also specifically tests that cover your addition (freezer-less cgroup destruction). src/linux/cgroups.cpp (lines 1626 - 1632) <https://reviews.apache.org/r/36620/#comment151417> Why would the freezer component not be available -- can you please mention that e.g. a root owned cgroup wont have freezer.state available? Not sure if there are other expected reasons for a missing freezer though. src/linux/cgroups.cpp (lines 1696 - 1697) <https://reviews.apache.org/r/36620/#comment151406> Kill one line plz. src/linux/cgroups.cpp (lines 1735 - 1737) <https://reviews.apache.org/r/36620/#comment151409> Wouldn't this be a bit better in terms of preventing jaggedness? ``` promise.fail( "Failed to kill all processes in cgroup: " + (processes.isError() ? processes.error() : "processes remain")); ``` src/linux/cgroups.cpp (line 1778) <https://reviews.apache.org/r/36620/#comment151407> s/freezerCheckError/verify/ src/linux/cgroups.cpp (line 1854) <https://reviews.apache.org/r/36620/#comment151408> s/move/Move/ - Till Toenshoff On Aug. 24, 2015, 9:33 a.m., Joerg Schad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36620/ > ----------------------------------------------------------- > > (Updated Aug. 24, 2015, 9:33 a.m.) > > > Review request for mesos, Benjamin Hindman, Till Toenshoff, and Timothy Chen. > > > Bugs: MESOS-3086 > https://issues.apache.org/jira/browse/MESOS-3086 > > > Repository: mesos > > > Description > ------- > > WIP Added Non-Freezeer Task Killer. > > > Diffs > ----- > > src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f > > Diff: https://reviews.apache.org/r/36620/diff/ > > > Testing > ------- > > sudo make check > + manual tests > > > Thanks, > > Joerg Schad > >
