mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevin Klues <klue...@gmail.com>
Subject Re: Review Request 51407: Updated mesos containerizer to checkpoint container runtime information.
Date Sat, 24 Sep 2016 19:14:41 GMT


> On Sept. 18, 2016, 11:51 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 1794-1800
> > <https://reviews.apache.org/r/51407/diff/6/?file=1501288#file1501288line1794>
> >
> >     Hum, should we do the deletion here if we don't even sure the processes are
killed properly?
> >     
> >     I think my suggestion on RuntimePath for container is:
> >     1) we create the RuntimePath as the first thing in 'launch', even before we
call any provisioner/isolator functions.
> >     2) we checkpoint the pid right after fork
> >     3) we delete the RuntimePath after the destroy is successful.
> >     
> >     The invariants we have if using the above way are:
> >     1) If RuntimePath exists, we know that provisioner/isolator prepare might be
called, so cleanup is necessary during recovery.
> >     2) If RuntimePath does not exist, we know that all cleanups have been done properly
and we no longer need to worry about cleanup.
> >     3) If pid file exists, we know that the process has been forked.
> >     4) If the pid file does not exist, we may or maynot have process being forked.
> >     
> >     For the upgrade situation, some checkpointed containers or orphans may not have
RuntimePath. In such case, we should not create RuntimePath (i.e., do not checkpoint pid or
exit status) in order to maintain the above invariant. So for old containers launched by previous
version of agent, there will be no RuntimePath for it at any time (this is another invariant).
> >     
> >     It's likely that a container has RuntimePath, but launcher does not know about
it. This is the case where launcher->destroy has been called (and being successful), but
agent crashes before removing RuntimePath.
> >     
> >     It's also likely that a container does not have RuntimePath, but launcher knows
about it. This is the legacy container case.
> 
> Jie Yu wrote:
>     Hum, I don't see all the comments here are addressed? Especially the following:
>     
>     > 1) we create the RuntimePath as the first thing in 'launch', even before we
call any provisioner/isolator functions
>     
>     > For the upgrade situation, some checkpointed containers or orphans may not have
RuntimePath. In such case, we should not create RuntimePath (i.e., do not checkpoint pid or
exit status) in order to maintain the above invariant.

I don't see the difference between creating the runtime directory before the provisioner/isolator
calls and doing it just before the fork. Mostly because I don't believe (2):

2) If RuntimePath does not exist, we know that all cleanups have been done properly and we
no longer need to worry about cleanup.

It's possible with legacy containers for the runtime path to not exist, but we still have
to do cleanup.

The invariants I saw for putting it where I did (i.e. just before the fork call) were the
following:
Note: These invariants are only really important when doing recovery.

1) If the runtime directory exists, but does not have a pid in it:
   -- We know the container is dead (it may have started running, but will die naturally because
it never got unblocked by the agent pipe).
   -- The launch.recover() may or may not return it as a known orphan depending on if it actually
got to this point or not.
2) If the runtime directory exists, and it does have a pid in it:
   -- The container was definitely started, but it may or may not be running.
   -- The launch.recover() will definitely return it as a known orphan
3) If the runtime directory does not exist:
   a) We are dealing with a legacy container
   b) We never even forked a new-style container
   Since we can't distinguish between these two cases, we have to go through the normal recover
path no matter what.
   
I guess your reasoning is that in 3b, even though we have to still call cleanup on everything,
it will basically be a no-op because we never would have called prepare on the provisioner/isolator
to begin with?


- Kevin


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


On Sept. 23, 2016, 9:01 p.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51407/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2016, 9:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6204
>     https://issues.apache.org/jira/browse/MESOS-6204
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This includes checkpointing both the container pid and the status of
> the container upon exit. This also includes an update to tests to
> account for new 'init' process semantics in a container. That is, the
> name of the init process of the container is now "mesos-containerizer"
> not "sh".
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 16f9e3e92e90fe7f8a0ebd24e567800e1f285bc9

>   src/slave/containerizer/mesos/containerizer.cpp 144b0db501d40d4e0bba12672723616bedd76e7e

>   src/tests/containerizer/isolator_tests.cpp b4d25e57df7f0e157769c9ae4f7847657c505e78

> 
> Diff: https://reviews.apache.org/r/51407/diff/
> 
> 
> Testing
> -------
> 
> $ GTEST_FILTER="" make -j check
> $ src/mesos-tests
> $ sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message