----------------------------------------------------------- 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 > >
