mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 35927: Added TaskStatus::Reason to Termination Message.
Date Mon, 29 Jun 2015 19:40:46 GMT

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


Thanks Joerg! See my detailed comments. I suggest you take a look at the discussion in https://reviews.apache.org/r/34720/
first.


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

    Not sure if that's needed. I think REASON_CONTAINER_LAUNCH_ERROR with proper message should
be sufficient? cc @tnachen



src/slave/containerizer/docker.cpp (line 1301)
<https://reviews.apache.org/r/35927/#comment142585>

    See my above comments. The slave is the one that should set executor->reason to be
REASON_CONTAINER_LAUNCH_ERROR. The containerizer just need to properly set the messages.



src/slave/containerizer/mesos/containerizer.cpp (line 1152)
<https://reviews.apache.org/r/35927/#comment142586>

    We could follow up with patches to remove 'killed' in both docker and mesos containerizer.



src/slave/slave.cpp (lines 4452 - 4469)
<https://reviews.apache.org/r/35927/#comment142584>

    Please take a look at the discussion in this review (2nd comments)
    https://reviews.apache.org/r/34720/
    
    There's some subtleness involved for command executors.
    
    The slave also needs to set executor->reason to be  REASON_CONTAINER_LAUNCH_ERROR when
containerizer->launch returns a failure.


- Jie Yu


On June 29, 2015, 7:22 a.m., Joerg Schad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
> 
> (Updated June 29, 2015, 7:22 a.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 b8591116eadcd68b8db2a629fbcf793e6b394f14 
>   src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e 
> 
> Diff: https://reviews.apache.org/r/35927/diff/
> 
> 
> Testing
> -------
> 
> make check (including docker tests)
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message