You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Till Toenshoff <to...@me.com> on 2014/05/20 04:09:16 UTC

Review Request 21677: Added workaround to allow the ExternalContainerizer containers to get recovered via slave state.

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

Review request for mesos.


Repository: mesos-git


Description
-------

The slave currently expects a forked executor PID within its states when recovering. For the ExternalContainerizer however this does not make sense but is introduced in this patch to get around the problem as described in MESOS-1328 and MESOS-923. This patch can entirely be dropped if a fix for the mentioned tickets comes in first.


Diffs
-----

  src/slave/containerizer/external_containerizer.cpp f9811c4 

Diff: https://reviews.apache.org/r/21677/diff/


Testing
-------

make check (in upcoming EC test suite)


Thanks,

Till Toenshoff


Re: Review Request 21677: Added workaround to allow the ExternalContainerizer containers to get recovered via slave state.

Posted by Till Toenshoff <to...@me.com>.

> On May 23, 2014, 8:27 p.m., Niklas Nielsen wrote:
> > src/slave/containerizer/external_containerizer.cpp, lines 442-444
> > <https://reviews.apache.org/r/21677/diff/1/?file=584874#file584874line442>
> >
> >     Can you be more precise here? It's great that you reference the JIRA, but the real issue is that containerizer(s) (so far) has relied on state being persisted in the slave. However, that responsibility should have been delegated to the containerizer too?

That is correct, the (mesos) containerizer should persist the forked pid if it needs that for recover. I will try to clear that up a bit.


- Till


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


On May 20, 2014, 2:09 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21677/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 2:09 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The slave currently expects a forked executor PID within its states when recovering. For the ExternalContainerizer however this does not make sense but is introduced in this patch to get around the problem as described in MESOS-1328 and MESOS-923. This patch can entirely be dropped if a fix for the mentioned tickets comes in first.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/external_containerizer.cpp f9811c4 
> 
> Diff: https://reviews.apache.org/r/21677/diff/
> 
> 
> Testing
> -------
> 
> make check (in upcoming EC test suite)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 21677: Added workaround to allow the ExternalContainerizer containers to get recovered via slave state.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21677/#review43873
-----------------------------------------------------------

Ship it!



src/slave/containerizer/external_containerizer.cpp
<https://reviews.apache.org/r/21677/#comment78151>

    Can you be more precise here? It's great that you reference the JIRA, but the real issue is that containerizer(s) (so far) has relied on state being persisted in the slave. However, that responsibility should have been delegated to the containerizer too?


- Niklas Nielsen


On May 19, 2014, 7:09 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21677/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 7:09 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The slave currently expects a forked executor PID within its states when recovering. For the ExternalContainerizer however this does not make sense but is introduced in this patch to get around the problem as described in MESOS-1328 and MESOS-923. This patch can entirely be dropped if a fix for the mentioned tickets comes in first.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/external_containerizer.cpp f9811c4 
> 
> Diff: https://reviews.apache.org/r/21677/diff/
> 
> 
> Testing
> -------
> 
> make check (in upcoming EC test suite)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


Re: Review Request 21677: Added workaround to allow the ExternalContainerizer containers to get recovered via slave state.

Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21677/
-----------------------------------------------------------

(Updated May 23, 2014, 11:06 p.m.)


Review request for mesos.


Changes
-------

Rephrased comment.


Repository: mesos-git


Description
-------

The slave currently expects a forked executor PID within its states when recovering. For the ExternalContainerizer however this does not make sense but is introduced in this patch to get around the problem as described in MESOS-1328 and MESOS-923. This patch can entirely be dropped if a fix for the mentioned tickets comes in first.


Diffs (updated)
-----

  src/slave/containerizer/external_containerizer.cpp ac3dd18 

Diff: https://reviews.apache.org/r/21677/diff/


Testing
-------

make check (in upcoming EC test suite)


Thanks,

Till Toenshoff