Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8/#review203778 --- Ship it! Ship It! - Greg Mann On April 17, 2018, 3:23 p.m.,

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 24, 2018, 4:24 p.m., Qian Zhang wrote: > > Ship It! A minor comment, since we fixed a tech debt (memory leak) in `ComposingContainerizerProcess::__recover()` in this patch, can you please mention it in the commit message? - Qian

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8/#review203734 --- Ship it! Ship It! - Qian Zhang On April 17, 2018, 11:23 p.m.

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 21, 2018, 1:21 p.m., Qian Zhang wrote: > > Can you please explain how the container will be cleaned up from the > > `containers_` map after agent recovery? > > Andrei Budnik wrote: > In the current implementation, a recovered container can be cleaned up > only when someone calls `

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 21, 2018, 1:21 p.m., Qian Zhang wrote: > > src/slave/containerizer/composing.cpp > > Lines 396-400 (original) > > > > > > Previously, in the case that `destroy-in-progress stopped an > > launch-in-progress`

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 21, 2018, 9:21 p.m., Qian Zhang wrote: > > Can you please explain how the container will be cleaned up from the > > `containers_` map after agent recovery? > > Andrei Budnik wrote: > In the current implementation, a recovered container can be cleaned up > only when someone calls `

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 21, 2018, 1:21 p.m., Qian Zhang wrote: > > Can you please explain how the container will be cleaned up from the > > `containers_` map after agent recovery? In the current implementation, a recovered container can be cleaned up only when someone calls `destroy()`. I didn't change that

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 21, 2018, 1:21 p.m., Qian Zhang wrote: > > src/slave/containerizer/composing.cpp > > Lines 396-400 (original) > > > > > > Previously, in the case that `destroy-in-progress stopped an > > launch-in-progress`

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 21, 2018, 1:21 p.m., Qian Zhang wrote: > > src/slave/containerizer/composing.cpp > > Line 361 (original), 360-365 (patched) > > > > > > So besides removing the container from the `containers_` map, we also

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 19, 2018, 1:26 a.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 669 (original), 618 (patched) > > > > > > Why return `wait()` when the state is DESTROYING, rather than just > > forw

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 18, 2018, 5:26 p.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 669 (original), 618 (patched) > > > > > > Why return `wait()` when the state is DESTROYING, rather than just > > forw

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 18, 2018, 5:26 p.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 669 (original), 618 (patched) > > > > > > Why return `wait()` when the state is DESTROYING, rather than just > > forw

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8/#review203490 --- Can you please explain how the container will be cleaned up from t

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 19, 2018, 1:26 a.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 669 (original), 618 (patched) > > > > > > Why return `wait()` when the state is DESTROYING, rather than just > > forw

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 18, 2018, 5:26 p.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 669 (original), 618 (patched) > > > > > > Why return `wait()` when the state is DESTROYING, rather than just > > forw

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8/#review203428 --- src/slave/containerizer/composing.cpp Line 669 (original), 618 (p

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 15, 2018, 11:05 p.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 361 (original), 360-365 (patched) > > > > > > Moving our previous discussion from https://reviews.apache.org/r/9/

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 15, 2018, 11:05 p.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 361 (original), 360-365 (patched) > > > > > > Moving our previous discussion from https://reviews.apache.org/r/9/

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 15, 2018, 11:05 p.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 361 (original), 360-365 (patched) > > > > > > Moving our previous discussion from https://reviews.apache.org/r/9/

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 15, 2018, 11:05 p.m., Greg Mann wrote: > > src/slave/containerizer/composing.cpp > > Line 361 (original), 360-365 (patched) > > > > > > Moving our previous discussion from https://reviews.apache.org/r/9/

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8/#review203185 --- src/slave/containerizer/composing.cpp Line 361 (original), 360-36

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 9, 2018, 3:15 p.m., Qian Zhang wrote: > > src/slave/containerizer/composing.cpp > > Lines 658-661 (original) > > > > > > If we remove these codes, will the container be missed to delete and > > erased from

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 9, 2018, 3:15 p.m., Qian Zhang wrote: > > I checked the callers of `Containerizer::destroy()`, it seems no one > > actually cares about its return value (`Option`), so > > why do we need to return that? Can we just make it return `Future`? > > Qian Zhang wrote: > And then for the

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 9, 2018, 11:15 p.m., Qian Zhang wrote: > > src/slave/containerizer/composing.cpp > > Lines 658-661 (original) > > > > > > If we remove these codes, will the container be missed to delete and > > erased from

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

> On May 9, 2018, 11:15 p.m., Qian Zhang wrote: > > I checked the callers of `Containerizer::destroy()`, it seems no one > > actually cares about its return value (`Option`), so > > why do we need to return that? Can we just make it return `Future`? And then for the last line of `ComposingCont

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8/#review202746 --- I checked the callers of `Containerizer::destroy()`, it seems no o