> On May 9, 2018, 3:15 p.m., Qian Zhang wrote:
> > I checked the callers of `Containerizer::destroy()`, it seems no one
> > actually cares about its return value (`Option<ContainerTermination>`), so
> > why do we need to return that? Can we just make it return `Future<Nothing>`?
>
> Qian Zhang wrote:
> And then for the last line of `ComposingContainerizerProcess::destroy`:
> ```
> return container->containerizer->wait(containerId);
> ```
> We could change it to something like:
> ```
> return container->containerizer->wait(containerId)
> .onAny([](const Future<Option<ContainerTermination>>& termination)
> {return Nothing();})
> ```
There are plans to use the return value of `destroy()` in tests, see
[MESOS-8829](https://issues.apache.org/jira/browse/MESOS-8829).
Also, as `destroy()` depends on `wait()` method, it'd be necessary to implement
wrappers like your example above, if we want them to return different type.
Those wrappers introduce some extra complexity, which IMO doesn't facilitate
achieving one of the goals "to simplify composing containerizer".
> On May 9, 2018, 3:15 p.m., Qian Zhang wrote:
> > src/slave/containerizer/composing.cpp
> > Lines 658-661 (original)
> > <https://reviews.apache.org/r/66668/diff/2/?file=2012420#file2012420line658>
> >
> > If we remove these codes, will the container be missed to delete and
> > erased from `containers_`?
>
> Qian Zhang wrote:
> Ah, Just saw the code to delete it in the next patch. Anyway I think it
> may be better to put that change into this patch :-)
`container_` cleanup is implemented in the following patch `/r/66669/`.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66668/#review202746
-----------------------------------------------------------
On April 17, 2018, 3:23 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66668/
> -----------------------------------------------------------
>
> (Updated April 17, 2018, 3:23 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian
> Zhang.
>
>
> Bugs: MESOS-8712
> https://issues.apache.org/jira/browse/MESOS-8712
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, we were using `destroyed` promise for each container to
> guarantee that calling `destroy()` on a container returns a non-empty
> value when the destroy-in-progress stops an launch-in-progress using
> the next containerizer. Since we are unifying the semantics of the
> return type for both `wait()` and `destroy()` operations, we can
> remove the `destroyed` promise.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.cpp
> 186102c66d373dcd799cadd9fed7d1c8cb894971
>
>
> Diff: https://reviews.apache.org/r/66668/diff/2/
>
>
> Testing
> -------
>
> internal CI
>
>
> Thanks,
>
> Andrei Budnik
>
>
