mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Toenshoff" <toensh...@me.com>
Subject Re: Review Request 17567: Added External Containerizer.
Date Thu, 03 Apr 2014 01:56:17 GMT


> On March 31, 2014, 8:03 p.m., Vinod Kone wrote:
> > src/slave/containerizer/external_containerizer.cpp, lines 293-295
> > <https://reviews.apache.org/r/17567/diff/15/?file=531801#file531801line293>
> >
> >     This seems a bit unfortunate. How about creating a protobuf that wraps all the
arguments that are needed for launch (and other methods) and passing it on to the external
containerizer?

Packed this into ExternalTask which wraps TaskInfo and the mesos-executor path.


> On March 31, 2014, 8:03 p.m., Vinod Kone wrote:
> > src/slave/containerizer/external_containerizer.cpp, lines 1125-1126
> > <https://reviews.apache.org/r/17567/diff/15/?file=531801#file531801line1125>
> >
> >     Instead of closing the file descriptor, how about just writing the length of
the protobuf first and then serialized protobuf. That way the read end could know the exact
size of data to read.
> >     
> >     This is how we do serialization and deserialization of protobufs to files. See
protobuf::write() and protobuf::read().
> >     
> >     
> >
> 
> Till Toenshoff wrote:
>     I like this approach. Will play with this a bit to see if there is something we are
missing.

Now implemented that in the latest revision.


> On March 31, 2014, 8:03 p.m., Vinod Kone wrote:
> > src/slave/containerizer/external_containerizer.cpp, line 1135
> > <https://reviews.apache.org/r/17567/diff/15/?file=531801#file531801line1135>
> >
> >     If we do the above and if we have MESOS-995, can you just leverage subprocess
here?
> >
> 
> Till Toenshoff wrote:
>     Almost, yes. Let me quote from the header:
>       // TODO(tillt): Remove and use process::subprocess once that has
>       // become flexible enough to support all features needed:
>       // - in-child-context commands (chdir) after fork and before exec.
>       // - direct execution without the invocation of /bin/sh.
>       // - external ownership of pipe file descriptors.
>     
>     Additionally, we are missing a setsid within the child-context to prevent a slave
suicide once the external containerizer process gets terminated.
>     
>     For the reasoning:
>     chdir() is clear and would be covered by MESOS-995, which is great.
>     
>     Direct execution is needed as we currently are depending on proper pipe close escalation.
If /bin/sh is used to invoke and when its child process is closing a pipe, the shell has to
close its end as well. In some tests I did, I found out that this is not always happening,
depending on the shell in use - specifically dash on ubuntu appears to not do that in certain
configurations.
>     
>     External ownership of the pipe handles is needed as the current way of signaling
a complete transmission of those protobufs is done via detection of an EOF. Subprocess assumes
to have full control and does assume the pipes to be open until the process terminates.
>     
>     
>     Now circling back to your earlier comment on prepending the protobufs by their size
to signal a complete transmission. That might ease the process as we could remove the need
for "direct execution" and "external pipe ownership". So the only remaining reason to not
use subprocess then would be the missing setsid().
>

So now we are only missing setsid() to fully rely upon subprocess. Will propose an overload
as soon as I addressed all other comments.


- Till


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


On April 2, 2014, 2:02 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17567/
> -----------------------------------------------------------
> 
> (Updated April 2, 2014, 2:02 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-816
>     https://issues.apache.org/jira/browse/MESOS-816
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This patch adds the so-called external containerizer. This
> containerizer delegates all containerizer calls directly to 
> an external containerizer program (which can be specified on 
> start-up). Few calls have internal fall-back implementations 
> such as wait(), destroy() and usage().
> 
> The protocol for the interactions with the external program
> is as follows:
> 
> COMMAND (ADDITIONAL-PARAMETERS) < INPUT-PROTO > RESULT-PROTO
> 
> launch (ContainerID, --mesos-executor, <path>) < TaskInfo > ExternalStatus
> update (ContainerID) < ResourceArray > ExternalStatus
> usage (ContainerID) > ResourceStatistics
> wait (ContainerID) > ExternalTermination
> destroy (ContainerID) > ExternalStatus
> 
> When protocol buffers need to be provided, the Mesos side of
> the external containerizer implementation will serialize the 
> protos on stdin and vice-versa for reading protos on stdout as 
> drafted in the above scheme.
> 
> 
> NOTE: This is currently work in progress to cover all comments.
> 
> 
> Diffs
> -----
> 
>   configure.ac 5404dc2 
>   include/mesos/mesos.proto 37f8a7f 
>   src/Makefile.am 0775a0d 
>   src/examples/python/test-containerizer.in PRE-CREATION 
>   src/examples/python/test_containerizer.py PRE-CREATION 
>   src/slave/containerizer/containerizer.cpp 6de091e 
>   src/slave/containerizer/external_containerizer.hpp PRE-CREATION 
>   src/slave/containerizer/external_containerizer.cpp PRE-CREATION 
>   src/slave/flags.hpp c9a627b 
>   src/tests/external_containerizer_test.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/17567/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


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