-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51406/#review146819
-----------------------------------------------------------




src/slave/containerizer/mesos/launcher.cpp (lines 91 - 114)
<https://reviews.apache.org/r/51406/#comment213525>

    No need to use os::open. Simply call os::read to get the string from the 
file. For such IO operations, no need to make it async (e.g., LOG(INFO) is 
synchrounous as well).
    
    BTW: you forgot to close the fd.


- Jie Yu


On Aug. 25, 2016, 1:21 a.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51406/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2016, 1:21 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6088
>     https://issues.apache.org/jira/browse/MESOS-6088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously the 'mesos-containerizer launch' binary would simply exec
> into the actual command we wanted to launch after doing some set of
> preperatory work. The problem with this approach, however, is that
> this gives us no opportunity to checkpoint the exit status of the
> command so the agent can recover it in cases where it is offline at
> the time the command completes.
> 
> To compensate for this, we now allow 'mesos-containerizer launch' to
> take an optional '--exit_status_path' parameter, which indicates where
> the exit status of our command should be checkpointed. In order to
> do this checkpointing, however, we cannot simply exec into the command
> anymore. Instead we now fork-exec the command and reap it once it
> completes. We then checkpoint its exit status and return it as our
> own. We call the original process the 'init' process of the container
> and the fork-exec'd process its child.  As a side effect of doing
> things this way, we also have to be careful to forward all signals
> sent to the init process down to the child.
> 
> In order to properly reap the 'init' process, we also introduce a new
> 'Launcher::wait' command that knows how to reap the 'init' process
> directly and then retreive its exit status from the checkpoint (if
> necessary).
> 
> In a subsequent commit we will update the mesos containerizer to use
> this new functionality.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/launch.hpp 
> 0e86da9c7bd9c7fbedd7102d66b902d1c10e5e0b 
>   src/slave/containerizer/mesos/launch.cpp 
> 2db8db5ac2f4614d406b950a1b4e8098a0d90e7b 
>   src/slave/containerizer/mesos/launcher.hpp 
> 0eae09515d550a2c71ae1414d4a22943f1d09db9 
>   src/slave/containerizer/mesos/launcher.cpp 
> 413a8afdc56127a58c9599c436d17d6c98e62434 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 8fbe1e9742df85b0fc6de46ac81a0c064c845a63 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 7377316776646e3d660086da3c0d5b494ce8ace4 
>   src/tests/containerizer/launcher.hpp 
> 94c62b761a17436841bd19f3bf622cc8f1047876 
>   src/tests/containerizer/launcher.cpp 
> a92d9890f0931425d69ef8ce0896d081b8722079 
> 
> Diff: https://reviews.apache.org/r/51406/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Reply via email to