> On Aug. 27, 2015, 7:02 p.m., Jie Yu wrote: > > src/linux/cgroups.cpp, lines 1776-1791 > > <https://reviews.apache.org/r/36620/diff/13/?file=1039336#file1039336line1776> > > > > Could you please introduce a new function under cgroups namespace and > > put this logic there: > > > > ``` > > // Kill all processes in the given cgroup. > > Future<Nothing> cgroups::kill( > > const string& hierarchy, > > const string& cgroup) > > { > > ... > > if (freezerCheckError.isNone()) { > > } else { > > } > > > > return ...; > > } > > ``` > > > > You may want to rename the exsiting `cgroups::kill(hierarchy, cgroup, > > signal)` to `cgroups::signal`. > > Timothy Chen wrote: > Hi jie, thanks for chiming in. This should be ready after the comments > are addressed. I'll try to merge this as well when it's done. I'll ping Joerg > about this.
Hi Jie, I am not sure whether I should move the entire logic further into cgroups::kill. Both path (freezer vs Non-freezer) have distinct logic on how to destroy the cgroups, which in my understanding makes sense to have in seperate TasksKiller entities. Secondly cgroups::kill currently is little more than trying to kill the cgroup, when moving the logic in here cgroups::kill would have to call cgroups::freeze or call the iterative logic. Secondly, in my understanding the interface here is the destroy() call. I actually would like to refactor this piece of code in general, but this is outside the scope of this patch. If we want to discuss this further -or misunderstand something here- I would be happy to discuss it further! - Joerg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/#review96734 ----------------------------------------------------------- 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 > >
