> On Sept. 29, 2016, 8:08 p.m., Benjamin Mahler wrote: > > src/slave/containerizer/composing.cpp, lines 534-560 > > <https://reviews.apache.org/r/52330/diff/2/?file=1514328#file1514328line534> > > > > Would it be a bit simpler to do something like this? > > > > ``` > > if (launched) { > > if (container->state == LAUNCHING) { > > container->state = LAUNCHED; > > } > > } else { > > container->destroyed.set(false); > > containers_.erase(containerId); > > delete container; > > } > > > > return launched; > > ```
I could simplify. But I was trying to make it look as close as possible to top level container `_launch`. Makes it easy to make any changes in the future. > On Sept. 29, 2016, 8:08 p.m., Benjamin Mahler wrote: > > src/slave/containerizer/composing.cpp, lines 535-537 > > <https://reviews.apache.org/r/52330/diff/2/?file=1514328#file1514328line535> > > > > The first sentence just seems to be a written form of the logic here? fixed. > On Sept. 29, 2016, 8:08 p.m., Benjamin Mahler wrote: > > src/slave/containerizer/composing.cpp, lines 371-385 > > <https://reviews.apache.org/r/52330/diff/2/?file=1514328#file1514328line371> > > > > It looks like 3 of these comments say the same thing about none of the > > containerizers supporting the launch? fixed a bit. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52330/#review150917 ----------------------------------------------------------- On Sept. 29, 2016, 9:09 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52330/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2016, 9:09 p.m.) > > > Review request for mesos, Benjamin Mahler and Gilbert Song. > > > Bugs: MESOS-6260 > https://issues.apache.org/jira/browse/MESOS-6260 > > > Repository: mesos > > > Description > ------- > > Fixed the race between launch and destroy in composing containerizer. > > > Diffs > ----- > > src/slave/containerizer/composing.cpp > 1031326a81cbedb2659d8831a7f419a0944d40ad > src/tests/containerizer/composing_containerizer_tests.cpp > 631328ac54bd6c17abc82f2ea78dc2f5b5750253 > > Diff: https://reviews.apache.org/r/52330/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
