You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2012/06/22 07:47:39 UTC

Review Request: Add the cgroups isolation module.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/
-----------------------------------------------------------

Review request for mesos, Benjamin Hindman and Vinod Kone.


Description
-------

Add the cgroups isolation module.


Diffs
-----

  src/Makefile.am 8760f59 
  src/slave/cgroups_isolation_module.hpp PRE-CREATION 
  src/slave/cgroups_isolation_module.cpp PRE-CREATION 
  src/slave/isolation_module.cpp 5b7b4a2 

Diff: https://reviews.apache.org/r/5509/diff/


Testing
-------

Manual tests (including OOM tests).


Thanks,

Jie Yu

Re: Review Request: Add the cgroups isolation module.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/#review9613
-----------------------------------------------------------



src/slave/cgroups_isolation_module.hpp
<https://reviews.apache.org/r/5509/#comment20514>

    No need to make this virtual (it doesn't need to be virtual in ProcessBasedIsolationModule either).



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20507>

    s/DEFAULT_HIERARCHY/HIERARCHY/



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20509>

    Add a newline.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20508>

    s/DEFAULT_SUBSYSTEMS/SUBSYSTEMS/



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20510>

    I'd prefer you looped through and checked each individual subsystem. That way, when one isn't enabled the user can very clearly get some insight into which one. I'd also prefer to see a 'std::list<std::string>'  instead of a string with commas (which would require updating the cgroups::enabled and cgroups::busy API to take a list of string instead of a string with list semantics).
    
    The flip side of this will be being able to change which subsystems to actually use.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20511>

    Ditto comments above.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20512>

    A more informative comment please (i.e., why should they remove the directory?).



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20505>

    No need for the helpers, just have a 'subsystems' variable (and see comment above about it being a std::list<std::string>).



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20506>

    Ditto above.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20515>

    This should not be a member function, and you should give it a better name. ;)



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20516>

    It would be cool to have a map from resource name/type to subsystem. This would give you two things: the ability to change which subsystems you do isolation for on a per slave basis (amazing!) and a nice clean piece of code which loops through the map and performs the writeControl's as necessary. Assuming all the controls are "scalars" this might make it easy to add controls in the future as well (e.g., on the command line via something like --isolate=[disk, disk.subsystem]).



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20518>

    s/listen OOM/listen for OOM/



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20519>

    Ditto above.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20521>

    Kill this log message, add another one if future.isReady() is true if you'd like that says something along the lines of "Successfully destroyed cgroup for executor ...".



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20520>

    Is this not a fatal issue? Doesn't this mean that the executor/cgroup might keep running forever? 


- 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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/#review9225
-----------------------------------------------------------



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment19746>

    is 64 arbitrary or is there a reason? just curious



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment19745>

    why do u need to do a memset?



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment19747>

    use numify inside utils.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment19748>

    i see. so this is where 64 comes from. you should pull this out into a constant.



src/slave/cgroups_isolation_module.hpp
<https://reviews.apache.org/r/5509/#comment19749>

    use !infos.contains() here and everywhere else



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment19750>

    why an unnamed namespace?



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment19780>

    just do CHECK(os::user == "root") << ...



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment19781>

    ditto



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment19782>

    s/listen/listening/



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment19783>

    s/happens/happened/


- Vinod Kone


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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/#review9669
-----------------------------------------------------------



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment20627>

    Done.



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment20624>

    Done.



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment20628>

    Done.



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment20631>

    Done.



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment20632>

    Done.



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment20633>

    Done.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20634>

    Done.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20635>

    Done,



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20644>

    Done.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20642>

    Done.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20643>

    Done.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20636>

    Done.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20639>

    Done.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20640>

    Done.



src/slave/cgroups_isolation_module.hpp
<https://reviews.apache.org/r/5509/#comment20602>

    Done.



src/slave/cgroups_isolation_module.hpp
<https://reviews.apache.org/r/5509/#comment20603>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20604>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20605>

    Remove.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20606>

    Removed.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20607>

    Removed.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20608>

    I don't want to show the stack trace because it is not a bug.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20609>

    Same as above.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20610>

    Done. Refactored.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20611>

    Rafactored.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20612>

    Refactored.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20613>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20614>

    Removed.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20615>

    Removed.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20616>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20623>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20617>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20618>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20619>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20600>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20601>

    Done.


- Jie Yu


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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/#review9717
-----------------------------------------------------------



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20695>

    You have an invariant right now that 'resources' will always contain a Value for "cpus". Make that invariant explicit with a CHECK, or at least don't use Value::Scalar() as the "default" and instead call the version of 'get' that returns an Option and don't set the control if the resource doesn't end up existing (e.g., wrap the rest of the function body in a 'if (cpus.isSome())'). If you do the latter, please add a LOG(WARNING) when 'cpus.isNone()'.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20697>

    Ditto above.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20702>

    This probably should not be a CHECK (i.e., a processExited could occur before an oomWaited).
    
    In addition, it probably makes sense to "tag" each independent framework/executor cgroup so that we can distinguish old events that might fire after we have relaunched a new framework/executor that has the same ID values. I think it makes sense to put this "tag" in the cgroup name as well.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20700>

    s/destroy/destroyed/



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20699>

    Is there a specific reason why you did executor id first? I'd prefer framework then executor.


- Benjamin Hindman


On Aug. 1, 2012, 5:33 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5509/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 5:33 a.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/examples/utils.hpp 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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.

> On Aug. 1, 2012, 7:19 p.m., Benjamin Hindman wrote:
> > src/examples/balloon_executor.cpp, line 38
> > <https://reviews.apache.org/r/5509/diff/5/?file=131825#file131825line38>
> >
> >     Please get '64' from the ExecutorInfo in BallonExecutor::registered.

Done.


> On Aug. 1, 2012, 7:19 p.m., Benjamin Hindman wrote:
> > src/examples/balloon_framework.cpp, line 120
> > <https://reviews.apache.org/r/5509/diff/5/?file=131826#file131826line120>
> >
> >     Isn't TASK_FINISHED still an exceptional event? Don't you want to driver->abort for all status updates?

Abort for all states except TASK_FINISHED.


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/#review9707
-----------------------------------------------------------


On Aug. 1, 2012, 5:33 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5509/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 5:33 a.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/examples/utils.hpp 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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/#review9707
-----------------------------------------------------------



src/examples/balloon_executor.cpp
<https://reviews.apache.org/r/5509/#comment20688>

    Please get '64' from the ExecutorInfo in BallonExecutor::registered.



src/examples/balloon_framework.cpp
<https://reviews.apache.org/r/5509/#comment20689>

    Isn't TASK_FINISHED still an exceptional event? Don't you want to driver->abort for all status updates?


- Benjamin Hindman


On Aug. 1, 2012, 5:33 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5509/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 5:33 a.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/examples/utils.hpp 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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/#review9913
-----------------------------------------------------------

Ship it!


Ship It!

- Benjamin Hindman


On Aug. 2, 2012, 11:59 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5509/
> -----------------------------------------------------------
> 
> (Updated Aug. 2, 2012, 11:59 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/examples/utils.hpp PRE-CREATION 
>   src/slave/cgroups_isolation_module.hpp PRE-CREATION 
>   src/slave/cgroups_isolation_module.cpp PRE-CREATION 
>   src/slave/flags.hpp beb750e 
>   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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/
-----------------------------------------------------------

(Updated Aug. 2, 2012, 11:59 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Add flags in slave/flags.hpp so that we can configure cgroups_hierarchy_root from command line.


Description
-------

Add the cgroups isolation module.


Diffs (updated)
-----

  src/Makefile.am 10f1101 
  src/examples/balloon_executor.cpp PRE-CREATION 
  src/examples/balloon_framework.cpp PRE-CREATION 
  src/examples/utils.hpp PRE-CREATION 
  src/slave/cgroups_isolation_module.hpp PRE-CREATION 
  src/slave/cgroups_isolation_module.cpp PRE-CREATION 
  src/slave/flags.hpp beb750e 
  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

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/
-----------------------------------------------------------

(Updated Aug. 2, 2012, 6:15 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

1) Fixed a few comment typos.

2) Deal with the case in which the slave accidental reboots.


Description
-------

Add the cgroups isolation module.


Diffs (updated)
-----

  src/Makefile.am 10f1101 
  src/examples/balloon_executor.cpp PRE-CREATION 
  src/examples/balloon_framework.cpp PRE-CREATION 
  src/examples/utils.hpp 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

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/
-----------------------------------------------------------

(Updated Aug. 2, 2012, 1:30 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

More comments from Ben.


Description
-------

Add the cgroups isolation module.


Diffs (updated)
-----

  src/Makefile.am 10f1101 
  src/examples/balloon_executor.cpp PRE-CREATION 
  src/examples/balloon_framework.cpp PRE-CREATION 
  src/examples/utils.hpp 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

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/#review9725
-----------------------------------------------------------



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20707>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20708>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20720>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20706>

    Done.



src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/5509/#comment20705>

    Done.


- Jie Yu


On Aug. 1, 2012, 5:33 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5509/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 5:33 a.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/examples/utils.hpp 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
> 
>

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/
-----------------------------------------------------------

(Updated Aug. 1, 2012, 5:33 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Addressed Ben and Vinod's comments.


Description
-------

Add the cgroups isolation module.


Diffs (updated)
-----

  src/Makefile.am 10f1101 
  src/examples/balloon_executor.cpp PRE-CREATION 
  src/examples/balloon_framework.cpp PRE-CREATION 
  src/examples/utils.hpp 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

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
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.


Changes
-------

Updated to fit the new trunk.


Description
-------

Add the cgroups isolation module.


Diffs (updated)
-----

  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 (updated)
-------

make check (external test, including OOM tests).


Thanks,

Jie Yu

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/
-----------------------------------------------------------

(Updated June 22, 2012, 9:30 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Include an external unit test (using balloon framework).


Description
-------

Add the cgroups isolation module.


Diffs (updated)
-----

  src/Makefile.am 8760f59 
  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
-------

Manual tests (including OOM tests).


Thanks,

Jie Yu

Re: Review Request: Add the cgroups isolation module.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5509/
-----------------------------------------------------------

(Updated June 22, 2012, 7:27 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Cleanup the code. More nice info logs.


Description
-------

Add the cgroups isolation module.


Diffs (updated)
-----

  src/Makefile.am 8760f59 
  src/slave/cgroups_isolation_module.hpp PRE-CREATION 
  src/slave/cgroups_isolation_module.cpp PRE-CREATION 
  src/slave/isolation_module.cpp 5b7b4a2 

Diff: https://reviews.apache.org/r/5509/diff/


Testing
-------

Manual tests (including OOM tests).


Thanks,

Jie Yu