----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5401/#review8835 -----------------------------------------------------------
src/linux/cgroups.hpp <https://reviews.apache.org/r/5401/#comment18751> Let's just return a future. src/linux/cgroups.hpp <https://reviews.apache.org/r/5401/#comment18752> Ditto. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18763> A previous review added the process namespace, so you can clean this code up (and all other related reviews) and drop 'process::' everywhere (and s/std::string/string/ too iirc). src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18753> virtual src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18754> Also virtual. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18755> As in the other reviews, if you can get away with doing terminate where appropriate, and using initialize and finalize than you lower the complexity of the code. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18762> const & src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18764> Does this need to be a dispatch? src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18767> terminate src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18765> Does this need to be a dispatch? src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18761> const & src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18766> terminate src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18768> terminate src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18760> Do you need to dispatch? src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18759> terminate src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18758> terminate src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18757> terminate src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18756> Kill this. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18769> Again, if this stuff was passed into the constructor you can eliminate the extra dispatch. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18771> Move '*' next to type. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment18770> Ditto from 'freezeCgroup'. src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/5401/#comment18772> Put expected value first (here and everywhere else please). - Benjamin Hindman On June 21, 2012, 4:57 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5401/ > ----------------------------------------------------------- > > (Updated June 21, 2012, 4:57 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > This patch provides APIs for controlling the cgroups freezer subsystem. > > It is very useful when a slave wants to kill/pause all the processes in (or > forked by) an executor. > > For example, when an out-of-memory event happens, the slave could choose 1) > kill the executor, 2) pause the executor and notify the user, and so on. > > > Diffs > ----- > > src/linux/cgroups.hpp PRE-CREATION > src/linux/cgroups.cpp PRE-CREATION > src/tests/cgroups_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/5401/diff/ > > > Testing > ------- > > On Linux machine, make check. > > > Thanks, > > Jie Yu > >
