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



include/mesos/containerizer/containerizer.proto (lines 87 - 89)
<https://reviews.apache.org/r/35927/#comment142324>

    Why do we need a deprecation cycle for this? AFAIK, it's part of the 
containerizer API only and I'm not aware of any third-party containers.



include/mesos/containerizer/containerizer.proto (line 95)
<https://reviews.apache.org/r/35927/#comment142323>

    Add a space.



include/mesos/mesos.proto (lines 886 - 889)
<https://reviews.apache.org/r/35927/#comment142322>

    Let's order field tags: 19 -> ... -> 21



include/mesos/slave/isolator.hpp (line 51)
<https://reviews.apache.org/r/35927/#comment142325>

    Shouldn't you include `<mesos/mesos.hpp>` header?



include/mesos/slave/isolator.hpp (line 64)
<https://reviews.apache.org/r/35927/#comment142326>

    AFAIK, techinally, limitation does not necessarily end in termination, 
though it is so for `MesosContainerizer`.
    
    Also, I'm not sure that reusing `TaskStatus::Reason` enum is a good idea: 
it includes much more stuff that is not related to limitations.



src/slave/containerizer/isolators/cgroups/mem.cpp (line 696)
<https://reviews.apache.org/r/35927/#comment142329>

    Is there a test for this case? Or do you plan to add it as part of the WIP?



src/slave/containerizer/isolators/posix/disk.cpp (line 253)
<https://reviews.apache.org/r/35927/#comment142327>

    Is there a test for this case? Or do you plan to add it as part of the WIP?



src/tests/docker_containerizer_tests.cpp 
<https://reviews.apache.org/r/35927/#comment142330>

    Why removing `TODO` wihtout checking for reason?



src/tests/docker_containerizer_tests.cpp 
<https://reviews.apache.org/r/35927/#comment142331>

    ditto


- Alexander Rukletsov


On June 26, 2015, 4:26 p.m., Joerg Schad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
> 
> (Updated June 26, 2015, 4:26 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-2035
>     https://issues.apache.org/jira/browse/MESOS-2035
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> [WIP] 
>  -need to add documentation
>  -need to add test 
>  -not all potential paths are covered. It is not clear whether isolator sees 
> event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in 
> mem.cpp.
> 
> 
> Diffs
> -----
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   include/mesos/containerizer/containerizer.proto 
> f16ccc89f83da28c413ccfa0687a06b7515a605c 
>   include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> b0e343fdc7088b2895d5dc8bb416dbcbf241cae5 
>   src/slave/containerizer/isolators/posix/disk.cpp 
> b2f995cba36b1399db48af1de49d76c607f80abd 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 313e9b74d3a0157609226041246575d2c4a503f8 
>   src/slave/slave.cpp b3e1ccc28d2698d8bc91829d70787dc2fc17b80d 
>   src/tests/docker_containerizer_tests.cpp 
> 3a983c6813dab6fa03ccb2c87e1ea71866766d6e 
> 
> Diff: https://reviews.apache.org/r/35927/diff/
> 
> 
> Testing
> -------
> 
> make check (including docker tests)
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Reply via email to