----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5509/#review9610 -----------------------------------------------------------
src/examples/balloon_executor.cpp <https://reviews.apache.org/r/5509/#comment20491> Print what you're doing on std::cout so future users can more easily understand what's happening. src/examples/balloon_executor.cpp <https://reviews.apache.org/r/5509/#comment20490> I'm guessing you're doing a memset to make sure the memory actually gets paged in (touched) as considered by the OS. Just make a comment as such. src/examples/balloon_executor.cpp <https://reviews.apache.org/r/5509/#comment20492> If you don't use numify, please catch the possible exceptions. src/examples/balloon_executor.cpp <https://reviews.apache.org/r/5509/#comment20502> Do you not want to balloon forever? If not, please explain your rational here for posterity (e.g., perhaps you expect the isolation module to work on strict boundaries and you want to exactly test that boundary in which case you want the "ballon test" to fail in a non-fuzzy way). src/examples/balloon_executor.cpp <https://reviews.apache.org/r/5509/#comment20501> These lines of tasks should actually not ever get executed UNLESS the cgroups isolation module fails to kill the executor, please make a comment as such. src/examples/balloon_framework.cpp <https://reviews.apache.org/r/5509/#comment20493> Kill const. src/examples/balloon_framework.cpp <https://reviews.apache.org/r/5509/#comment20494> Just use a bool if you only want to launch one task. src/examples/balloon_framework.cpp <https://reviews.apache.org/r/5509/#comment20500> I think you are actually expecting a TASK_LOST status update, because the executor should fail once it uses too much memory and the slave should generate a TASK_LOST for you. If you get any other status update you should driver->abort() so that this process exits (below) with a non-zero status code. src/examples/balloon_framework.cpp <https://reviews.apache.org/r/5509/#comment20495> Please put this into examples/utils.hpp and update the other frameworks use it too! Awesome! src/examples/balloon_framework.cpp <https://reviews.apache.org/r/5509/#comment20496> This, however, should be const. src/examples/balloon_framework.cpp <https://reviews.apache.org/r/5509/#comment20497> s/ *m/* m/ src/examples/balloon_framework.cpp <https://reviews.apache.org/r/5509/#comment20498> +1 src/examples/balloon_framework.cpp <https://reviews.apache.org/r/5509/#comment20499> Do you want to do any checking to makes sure <balloon size in MB> is larger than 64 MB? - Benjamin Hindman On July 17, 2012, 7:49 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5509/ > ----------------------------------------------------------- > > (Updated July 17, 2012, 7:49 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > Add the cgroups isolation module. > > > Diffs > ----- > > src/Makefile.am 10f1101 > src/examples/balloon_executor.cpp PRE-CREATION > src/examples/balloon_framework.cpp PRE-CREATION > src/slave/cgroups_isolation_module.hpp PRE-CREATION > src/slave/cgroups_isolation_module.cpp PRE-CREATION > src/slave/isolation_module.cpp 5b7b4a2 > src/tests/cgroups_isolation_tests.cpp PRE-CREATION > src/tests/external/CgroupsIsolation/ROOT_CGROUPS_BalloonFramework.sh > PRE-CREATION > > Diff: https://reviews.apache.org/r/5509/diff/ > > > Testing > ------- > > make check (external test, including OOM tests). > > > Thanks, > > Jie Yu > >
