> 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 > >