----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66669/#review202868 -----------------------------------------------------------
src/slave/containerizer/composing.cpp Lines 488-502 (original), 493-512 (patched) <https://reviews.apache.org/r/66669/#comment284901> I'm wondering if, instead of also adding a callback for this in `destroy()`, perhaps we should unconditionally remove the container from the `containers_` map in the callbacks which are registered on `wait()`? i.e. we would do the following here: ``` if (launchResult == Containerizer::LaunchResult::SUCCESS) { // Note that we don't update the state if a destroy is in progress. if (container->state == LAUNCHING) { container->state = LAUNCHED; } // This is needed for eventually removing the given container from // the list of active containers. container->containerizer->wait(containerId) .onAny(defer(self(), [=](const Future<Option<ContainerTermination>>&) { if (containers_.contains(containerId)) { delete containers_.at(containerId); containers_.erase(containerId); } })); // Note that the return value is not impacted // by whether a destroy is currently in progress. return Containerizer::LaunchResult::SUCCESS; } ``` and NOT register the callback in `destroy()`. This might also mean that we could remove `if (containers_.contains(containerId))` and instead do `CHECK(containers_.contains(containerId));`? WDYT? - Greg Mann 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/66669/ > ----------------------------------------------------------- > > (Updated April 17, 2018, 3:23 p.m.) > > > Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian > Zhang. > > > Bugs: MESOS-8714 > https://issues.apache.org/jira/browse/MESOS-8714 > > > Repository: mesos > > > Description > ------- > > This patch adds callbacks on `wait()` and `destroy()` in composing > containerizer to remove a container from the `containers_` map after > the container's termination. > > > Diffs > ----- > > src/slave/containerizer/composing.cpp > 186102c66d373dcd799cadd9fed7d1c8cb894971 > > > Diff: https://reviews.apache.org/r/66669/diff/2/ > > > Testing > ------- > > internal CI > > > Thanks, > > Andrei Budnik > >
