----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5401/#review9399 -----------------------------------------------------------
src/linux/cgroups.hpp <https://reviews.apache.org/r/5401/#comment20129> Why not just put the default here then rather than use an Option? src/linux/cgroups.hpp <https://reviews.apache.org/r/5401/#comment20130> Same here as above. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment20132> Shouldn't need a default here. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment20131> You can kill this and just put the default in the header. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment20135> Replace CHECK(false) with LOG(FATAL). src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment20136> Same as above. src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment20133> You can make this const by setting it via a constructor initializer list. Also, why not use 'seconds' type? src/linux/cgroups.cpp <https://reviews.apache.org/r/5401/#comment20134> Why not use 'seconds' type? - Benjamin Hindman On July 16, 2012, 6:56 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5401/ > ----------------------------------------------------------- > > (Updated July 16, 2012, 6:56 p.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 cec9e36 > src/linux/cgroups.cpp 93c140f > src/tests/cgroups_tests.cpp 7f0f901 > > Diff: https://reviews.apache.org/r/5401/diff/ > > > Testing > ------- > > On Linux machine, make check. > > > Thanks, > > Jie Yu > >
