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 63063: Modified Containerizer::launch interface to allow repeated launch.
Date Wed, 08 Nov 2017 19:23:22 GMT

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


Fix it, then Ship it!





src/slave/containerizer/docker.cpp
Lines 1259-1260 (original), 1259-1271 (patched)
<https://reviews.apache.org/r/63063/#comment267861>

    This is a bit weird. I think the interface of `reapExecutor` is made in such a way so
that it can be chained.
    
    Can you change `reapExecutor`'s interface to return Nothing, and chain a lambda that returns
a SUCCESS
    ```
    return container->launch = fetch(containerId)
      ...
      .then(defer(self(), [=](pid_t pid) {
        return reapExecutor(containerId, pid);
      }))
      .then([]() {
        return Containerizer::LaunchResult::SUCCESS;
      }));
    ```



src/slave/containerizer/docker.cpp
Lines 1321-1330 (patched)
<https://reviews.apache.org/r/63063/#comment267862>

    Ditto here.


- Jie Yu


On Nov. 2, 2017, 4:02 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63063/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2017, 4:02 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
>     https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> There is some existing tech debt around requiring the caller of
> `Containerizer::launch` to call `Containerizer::destroy` if the launch
> fails (see MESOS-6214).  For nested and standalone containers, the
> side effect of this results in accidentally destroying running
> containers if you make the same call an even number of times.
> 
> For example, suppose the user launches a valid nested container
> with an ID of 'parent.child'. If the user issues the same call to
> launch 'parent.child' again, this second call will fail *and* will
> also destroy the first container.
> 
> This commit prevents repeated launch calls from destroying containers
> by changing the return value of `Containerizer::launch`.  There are
> now four possible return values:
>   * The launch succeeded.
>   * The standalone/nested container already exists.
>   * The given ContainerConfig is not supported.
>   * The launch failed.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f 
>   src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208 
>   src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842

>   src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
>   src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
>   src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b

>   src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5

>   src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 337083dbe60bba2d3773b785bdceeaf0b8fcd070 
>   src/tests/agent_container_api_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63063/diff/2/
> 
> 
> Testing
> -------
> 
> Requires the next review in the chain (test changes).
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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