mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam B" <a...@mesosphere.io>
Subject Re: Review Request 33257: Fixed recover tasks only by the intiated containerizer.
Date Thu, 23 Apr 2015 08:12:05 GMT

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


Looks reasonable to me, but I'm no containerizer expert.


src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/33257/#comment131596>

    Why can this be removed in 0.24? Why 0.24?
    Can you create a JIRA and target it to 0.24 so we don't forget?



src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/33257/#comment131597>

    `continue` seems unnecessary for this tiny loop. How about:
    ```cpp
    if (id.isSome()) {
      existingContainers.insert(id.get());
    }
    ```



src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/33257/#comment131598>

    s/it/its/



src/slave/slave.cpp
<https://reviews.apache.org/r/33257/#comment131594>

    s/recovering/to recover/


- Adam B


On April 20, 2015, 2:18 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33257/
> -----------------------------------------------------------
> 
> (Updated April 20, 2015, 2:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ian Downes, Jie Yu, and Till
Toenshoff.
> 
> 
> Bugs: MESOS-2601
>     https://issues.apache.org/jira/browse/MESOS-2601
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fixed recover tasks only by the intiated containerizer.
> Currently both mesos and docker containerizer recovers tasks that wasn't started by themselves.
> The proposed fix is to record the intended containerizer in the checkpointed executorInfo,
and reuse that information on recover to know if the containerizer should recover or not.
We are free to modify the executorInfo since it's not being used to relaunch any task.
> The external containerizer doesn't need to change since it is only recovering containers
that are returned by the containers script.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp 0d5289c626bdf22420759485f2146abfb6bdaffc 
>   src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
>   src/slave/containerizer/mesos/containerizer.cpp e4136095fca55637864f495098189ab3ad8d8fe7

>   src/slave/slave.cpp 8ec80ed26f338690e0a1e712065750ab77a724cd 
>   src/tests/containerizer_tests.cpp 5991aa628083dac7c5e8bf7ba297f4f9edeec05f 
>   src/tests/docker_containerizer_tests.cpp b119a174de79c2f98a0c575e6be2f59649f509ef 
> 
> Diff: https://reviews.apache.org/r/33257/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


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