> On July 26, 2017, 10:48 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/containerizer.cpp > > Lines 2599-2603 (patched) > > <https://reviews.apache.org/r/61089/diff/1/?file=1781597#file1781597line2599> > > > > Would suggest we either > > 1. add a check here: `CHECK(containers_.contains(containerId)` > > > > or > > > > 2. pass an `const Owned<Container>` pointer to transition() instead of > > the `ContainerID`. > > > > Thoughts?
I would prefer the former. Thanks for the suggestion. The problem with the latter is that - I'd like to log the `containerId` which is not in the `Container` but if I pass both, the API looks kind of stupid. Adding `containerId` to `Container` would probably overkill. - Passing `Owned` to a method when you don't mean to pass on the ownership feels odd. Our pratices on `Owned` has always been murky but when possible I am sticking with the strict semantics. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61089/#review181472 ----------------------------------------------------------- On July 24, 2017, 2:48 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61089/ > ----------------------------------------------------------- > > (Updated July 24, 2017, 2:48 p.m.) > > > Review request for mesos and Gilbert Song. > > > Repository: mesos > > > Description > ------- > > Currently when container launch is blocked silently, it's hard to tell > which state it is in. Logging state transition helps with triaging. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.hpp > ea0691945d3450fcc336df03d9cf7b48afbd8b3d > src/slave/containerizer/mesos/containerizer.cpp > 9376d14d66f5dc7e91c7c0e9da253f5eb9347539 > > > Diff: https://reviews.apache.org/r/61089/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Jiang Yan Xu > >
