----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52330/#review150675 -----------------------------------------------------------
src/slave/containerizer/composing.cpp (lines 352 - 356) <https://reviews.apache.org/r/52330/#comment218709> Hm.. it doesn't seem correct to return satisfy the destroy future and clean up here. If launch succeeded and we're in DESTROYING we can just wait for the destroy to complete and satisfy the future according to the actual destroy result, no? src/tests/containerizer/composing_containerizer_tests.cpp (line 126) <https://reviews.apache.org/r/52330/#comment218705> Seems like "same as the above test" is prone to become stale if additional tests are added or the above test is changed? src/tests/containerizer/composing_containerizer_tests.cpp (lines 127 - 128) <https://reviews.apache.org/r/52330/#comment218706> 'this' means the destroy? The wording here seems a bit inaccurate? It seems like the destroy doesn't stop the loop, since the first one was able to launch the container. Maybe something like `DestroyDuringSuccessfulLaunch`? src/tests/containerizer/composing_containerizer_tests.cpp (lines 184 - 188) <https://reviews.apache.org/r/52330/#comment218708> Hm.. this seems wrong? It seems like if destroy returns false (or fails) then the destroyed future should surface as false (or fail) as well, since the underlying containerizer was not able to find (or destroy) the container. - Benjamin Mahler On Sept. 28, 2016, 3:15 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52330/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2016, 3:15 a.m.) > > > Review request for mesos, Benjamin Mahler and Gilbert Song. > > > Repository: mesos > > > Description > ------- > > Fixed the race between launch and destroy in composing containerizer. > > > Diffs > ----- > > src/slave/containerizer/composing.cpp > 17eb34cf9764bd9475dee8cc8c894ede55058dd8 > src/tests/containerizer/composing_containerizer_tests.cpp > 631328ac54bd6c17abc82f2ea78dc2f5b5750253 > > Diff: https://reviews.apache.org/r/52330/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
