> On March 10, 2016, 8:47 p.m., Joris Van Remoortere wrote:
> > src/slave/containerizer/mesos/linux_launcher.cpp, line 301
> > <https://reviews.apache.org/r/44087/diff/4/?file=1279106#file1279106line301>
> >
> >     We now rely on the hook logic to do this right?
> >     Let's document this.

This is actually one thing I wanted to discuss with you: We do (here as well as 
in extendLifetime) a kill inside the hook as well as one from the subprocess 
logic calling the hook (on receiving an error). Are we sure that we are always 
killing the right process (as there might have been a new process with the same 
pid) when killing it from the subprocess?


> On March 10, 2016, 8:47 p.m., Joris Van Remoortere wrote:
> > src/slave/containerizer/mesos/linux_launcher.cpp, lines 298-302
> > <https://reviews.apache.org/r/44087/diff/4/?file=1279106#file1279106line298>
> >
> >     What should we do here now that the hook infrastructure is in place?
> >     Should we log an error as well as return one? or return a more 
> > meaningful error message?

See my comment below. In general I would prefere a logic where we just return 
an error (from the Hook) and the subprocess logic takes care of killing and 
logging.


- Joerg


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


On March 3, 2016, 3:39 p.m., Joerg Schad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44087/
> -----------------------------------------------------------
> 
> (Updated March 3, 2016, 3:39 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moved logic to assign process to freezer hierarchy into parentHook.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 9c80cfb621ef2e28aabfb2649846892964d2d4f3 
> 
> Diff: https://reviews.apache.org/r/44087/diff/
> 
> 
> Testing
> -------
> 
> sudo make check (on Linux)
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Reply via email to