> On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.hpp, line 253 > > <https://reviews.apache.org/r/5401/diff/4/?file=115068#file115068line253> > > > > Let's just return a future.
Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.hpp, line 267 > > <https://reviews.apache.org/r/5401/diff/4/?file=115068#file115068line267> > > > > Ditto. Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 761 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line761> > > > > 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). Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 769 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line769> > > > > virtual Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 771 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line771> > > > > Also virtual. Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 774 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line774> > > > > 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. Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 785 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line785> > > > > const & Done > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 795 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line795> > > > > Does this need to be a dispatch? Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 808 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line808> > > > > terminate Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 810 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line810> > > > > Does this need to be a dispatch? Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 815 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line815> > > > > const & Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 822 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line822> > > > > terminate Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 826 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line826> > > > > terminate Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 843 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line843> > > > > Do you need to dispatch? Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 849 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line849> > > > > terminate Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 861 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line861> > > > > terminate Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 865 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line865> > > > > terminate Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 872 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line872> > > > > Kill this. Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 911 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line911> > > > > Again, if this stuff was passed into the constructor you can eliminate > > the extra dispatch. Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 935 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line935> > > > > Move '*' next to type. Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/linux/cgroups.cpp, line 938 > > <https://reviews.apache.org/r/5401/diff/4/?file=115069#file115069line938> > > > > Ditto from 'freezeCgroup'. Done. > On July 3, 2012, 4:03 p.m., Benjamin Hindman wrote: > > src/tests/cgroups_tests.cpp, line 398 > > <https://reviews.apache.org/r/5401/diff/4/?file=115070#file115070line398> > > > > Put expected value first (here and everywhere else please). Done. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5401/#review8835 ----------------------------------------------------------- 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 > >
