> On April 3, 2016, 2:22 p.m., Guangya Liu wrote: > > src/slave/containerizer/docker.cpp, lines 794-797 > > <https://reviews.apache.org/r/44823/diff/4/?file=1323359#file1323359line794> > > > > const ExecutorInfo& executorInfo = recoverable.executor_info(); > > const ContainerID& containerId = recoverable.container_id(); > > const FrameworkID& frameworkId = executorInfo.framework_id(); > > const ExecutorID& executorId = executorInfo.executor_id();
Ditto here, see https://issues.apache.org/jira/browse/MESOS-2629 > On April 3, 2016, 2:22 p.m., Guangya Liu wrote: > > src/slave/containerizer/docker.cpp, lines 825-826 > > <https://reviews.apache.org/r/44823/diff/4/?file=1323359#file1323359line825> > > > > I think that here should be `||` but not `&&` Actually these code existed before. If change to `||`, those containers could be found in `existingContainers` and `executorInfo.has_container()` is false would not be recovered. > On April 3, 2016, 2:22 p.m., Guangya Liu wrote: > > src/slave/containerizer/docker.cpp, lines 825-839 > > <https://reviews.apache.org/r/44823/diff/4/?file=1323359#file1323359line825> > > > > I think that this logic can make the code more clear: > > > > if (!existingContainers.contains(containerId)) { > > xxx; > > continue; > > } > > > > if (!executorInfo.has_container()) { > > xxx; > > continue; > > } > > > > if (executorInfo.container().type() != ContainerInfo::DOCKER) { > > xxxx; > > continue; > > } Actually we could not skip recover when `!existingContainers.contains(containerId) && executorInfo.has_container()`. Below code would skip this case. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44823/#review126728 ----------------------------------------------------------- On April 3, 2016, 2:11 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44823/ > ----------------------------------------------------------- > > (Updated April 3, 2016, 2:11 p.m.) > > > Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Till > Toenshoff, and Timothy Chen. > > > Bugs: MESOS-5085 > https://issues.apache.org/jira/browse/MESOS-5085 > > > Repository: mesos > > > Description > ------- > > Remove `SlaveState` in `DockerContainerizer` during recover. > > > Diffs > ----- > > src/slave/containerizer/docker.hpp 89d450e10a84f24ddd46d517e2b4b46ab02c4fda > src/slave/containerizer/docker.cpp 9314d1f9e0b6077fe7c48b860783ab21acc48be6 > src/tests/containerizer/docker_containerizer_tests.cpp > b9c26b92c44e8ca718a5eb333855199bdf2a8e81 > > Diff: https://reviews.apache.org/r/44823/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
