----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63680/#review191106 -----------------------------------------------------------
src/slave/container_daemon.hpp Lines 40 (patched) <https://reviews.apache.org/r/63680/#comment268692> We typically don't use javadoc-style comments for internal classes. src/slave/container_daemon.hpp Lines 88-90 (patched) <https://reviews.apache.org/r/63680/#comment268698> Looks like the intent of this method is to return when a loop-breaking error occurs. You should probably mention this. And that the only way to retry (given the current interface) is to reconstruct the ContainerDaemon object. src/slave/container_daemon.hpp Lines 89 (patched) <https://reviews.apache.org/r/63680/#comment268694> s/discraded/discarded/ src/slave/container_daemon.cpp Lines 136-147 (patched) <https://reviews.apache.org/r/63680/#comment268693> Style nit: Single newline needed before each `if` statement. Also, try spacing the protobuf mutation like: ``` launchCall.mutable_launch_container() ->mutable_command()->CopyFrom(commandInfo.get()); ``` src/slave/container_daemon.cpp Lines 170 (patched) <https://reviews.apache.org/r/63680/#comment268695> s/contanier/container/ src/slave/container_daemon.cpp Lines 188-198 (patched) <https://reviews.apache.org/r/63680/#comment268704> Unless you have plans to add more than one call site for this helper method, you should just inline the contents into `launchContainer`. Same thing with `stopped` and `waitContainer`. src/slave/container_daemon.cpp Lines 216-219 (patched) <https://reviews.apache.org/r/63680/#comment268702> It doesn't seem necessary to have a mutex here, especially since all the calls are initiated from within this process. There are no other actors that can Launch or Kill. There's only one place where the Kill is used anyway. So there's no way for the mutex to come into play. When the `force` parameter is true, the control flow is basically linear: kill -> wait -> launch -> wait -> ... - Joseph Wu On Nov. 15, 2017, 1:21 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63680/ > ----------------------------------------------------------- > > (Updated Nov. 15, 2017, 1:21 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, and Joseph Wu. > > > Bugs: MESOS-8183 > https://issues.apache.org/jira/browse/MESOS-8183 > > > Repository: mesos > > > Description > ------- > > The `ContanierDaemon` class is responsible to monitor if a long-running > service running in a standalone container, and restart the service > container after it exits. It takes the following hook functions: > > * `postStartHook`: called after the container is launched every time. > * `postStopHook`: called after the container exits every time. > > `ContainerDaemon` does not manage the lifecycle of the contanier it > monitors, so the container persists across `ContainerDaemon`s. However, > it provides a `terminate()` method to completely shutdown the service > container. > > > Diffs > ----- > > src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 > src/slave/container_daemon.hpp PRE-CREATION > src/slave/container_daemon.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/63680/diff/4/ > > > Testing > ------- > > > Thanks, > > Chun-Hung Hsiao > >
