----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66668/#review203490 -----------------------------------------------------------
Can you please explain how the container will be cleaned up from the `containers_` map after agent recovery? src/slave/containerizer/composing.cpp Line 361 (original), 360-365 (patched) <https://reviews.apache.org/r/66668/#comment285749> So besides removing the container from the `containers_` map, we also eliminate a call to `ComposingContainerizerProcess::destroy()`, My understanding is we assume the underlying containerizers will always do its own destroy to clean up its internal data, so here in composing containerizer we just need to remove the container from the `containers_` map, right? src/slave/containerizer/composing.cpp Lines 396-400 (original) <https://reviews.apache.org/r/66668/#comment285784> Previously, in the case that `destroy-in-progress stopped an launch-in-progress`, the `destroy()` will return `ContainerTermination()` which is set here. But now with this implementation, it will return `None()` which is set by the underlying containerizer. This seems not correct to me, in this case, the destroy should considered as succeeded (return `ContainerTermination()`) rather than failed (return `None()`). src/slave/containerizer/composing.cpp Lines 401-402 (original), 395-396 (patched) <https://reviews.apache.org/r/66668/#comment285754> Do we need these two lines? Since there is a destroy going on, the container will be removed from `containers_` when the destroy is done. - Qian Zhang On April 17, 2018, 11: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, 11: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 stored `destroyed` promise for each container and used > it to guarantee that `destroy()` returns a non-empty value when the > destroy-in-progress stops an launch-in-progress using the next > containerizer. Since `wait()` and `destroy()` return the same > `ContainerTermination` value when called with the same ContainerID > argument, we can remove `destroyed` promise and add callbacks to > clean up `containers_` map instead. > > > Diffs > ----- > > src/slave/containerizer/composing.cpp > 1fb79f53b48154ecbd3b0165b6a8002c871e6dce > > > Diff: https://reviews.apache.org/r/66668/diff/4/ > > > Testing > ------- > > internal CI > > > Thanks, > > Andrei Budnik > >
