You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2013/12/10 03:23:10 UTC
Review Request 16149: Containerizer - launchers
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs
-----
src/slave/container/launcher.hpp PRE-CREATION
src/slave/container/launcher.cpp PRE-CREATION
src/slave/container/mesos_launcher.hpp PRE-CREATION
src/slave/container/mesos_launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review30240
-----------------------------------------------------------
Benh and I went over this together.
Who owns the process termination, the launcher? Is this implemented?
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment57900>
This seems like part of the containerizer since it delivers these to the slave, could this be Containerizer::Termination?
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment57901>
End comments with a period here and elsewhere.
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment57899>
Place the Launcher API first in this file.
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment57902>
Documentation please :)
src/slave/container/launcher.cpp
<https://reviews.apache.org/r/16149/#comment57903>
We don't use snake case here, is this coming from the flags?
src/slave/container/launcher.cpp
<https://reviews.apache.org/r/16149/#comment57904>
There doesn't seem to be a need for this to be asynchronous, yet. Keeping it simple for now would be nice.
src/slave/container/launcher.cpp
<https://reviews.apache.org/r/16149/#comment57905>
Please use Failure() here and elsewhere.
src/slave/container/mesos_launcher.hpp
<https://reviews.apache.org/r/16149/#comment57906>
Why is there a MesosLauncher? What will the inheritance here be used for?
The Forker hierarchy seems to be sufficient for capturing the different behavior.
src/slave/container/mesos_launcher.hpp
<https://reviews.apache.org/r/16149/#comment57907>
Place the { on a newline. Here and elsewhere.
src/slave/container/mesos_launcher.cpp
<https://reviews.apache.org/r/16149/#comment57909>
What if it is launching but not yet running?
src/slave/container/mesos_launcher.cpp
<https://reviews.apache.org/r/16149/#comment57910>
Is there going to be a cgroups version of this termination?
src/slave/container/mesos_launcher.cpp
<https://reviews.apache.org/r/16149/#comment57911>
Is this necessary given you've called killtree with groups=true?
- Ben Mahler
On Dec. 11, 2013, 4:06 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2013, 4:06 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/slave/container/launcher.hpp PRE-CREATION
> src/slave/container/launcher.cpp PRE-CREATION
> src/slave/container/mesos_launcher.hpp PRE-CREATION
> src/slave/container/mesos_launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review30683
-----------------------------------------------------------
As with my previous (and rather short) review - this is also a few issues that breaks compilation. Will follow up with style review as well here.
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment58774>
Should this be LinuxForker?
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment58776>
Is there a missing comma at the end of this line?
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment58779>
There is some parentheses / bracket mismatch here; Should it be:
'static_cast<void*>(const_cast<lambda::function<int()>* >(&child)))) == -1) {'
instead ?
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment58791>
Should this be 'static int _call(void* child)'?
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment58775>
How about using Bytes() from stout to generate your 8MB. Also, wouldn't it be the right way to pick that number up from the system?
src/slave/container/launcher.cpp
<https://reviews.apache.org/r/16149/#comment58780>
Does not match previous _child signature.
src/slave/container/launcher.cpp
<https://reviews.apache.org/r/16149/#comment58794>
Doesn't match signature, should be:
execute(const std::string& command,
const std::string& directory,
const Option<std::map<std::string, std::string> >& env,
const Option<std::string>& user)
src/slave/container/launcher.cpp
<https://reviews.apache.org/r/16149/#comment58795>
s/_//g
- Niklas Nielsen
On Dec. 19, 2013, 8:01 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Dec. 19, 2013, 8:01 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am 42dafbcd6e2d4f67d2705c5a2bea6ec6a0f4fcc1
> src/slave/container/launcher.hpp PRE-CREATION
> src/slave/container/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
> On Jan. 22, 2014, 3:47 a.m., Niklas Nielsen wrote:
> > src/slave/container/launcher.hpp, line 124
> > <https://reviews.apache.org/r/16149/diff/6/?file=425486#file425486line124>
> >
> > Can you make this const?
This code has been removed.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review32450
-----------------------------------------------------------
On Jan. 21, 2014, 7:34 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2014, 7:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/container/cgroups_launcher.hpp PRE-CREATION
> src/slave/container/cgroups_launcher.cpp PRE-CREATION
> src/slave/container/launcher.hpp PRE-CREATION
> src/slave/container/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review32450
-----------------------------------------------------------
Ship it!
Cool! Some style knits and a question on error reporting :)
src/Makefile.am
<https://reviews.apache.org/r/16149/#comment61295>
Can you align '\''s? Again, the tabstop is 8.
src/Makefile.am
<https://reviews.apache.org/r/16149/#comment61297>
And here.
src/slave/container/cgroups_launcher.hpp
<https://reviews.apache.org/r/16149/#comment61307>
Can hierarchy be made const?
src/slave/container/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment61309>
Can subsystem be made const?
src/slave/container/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment61298>
Should we do a more graceful exit? Like EXIT(1) << ...
src/slave/container/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment61310>
Again, any need for LOG(FATAL)?
src/slave/container/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment61306>
Again - any reason for LOG(FATAL) instead of EXIT(1)?
src/slave/container/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment61303>
What happens if cgroups::exists() fails? It looks like an already exist error message will be shown? I found a bit confusing when figuring out that cgroups was mounted elsewhere (/sys/fs/cgroup), but that might of course just be my setup.
src/slave/container/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment61304>
s/fark/fork/
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment61305>
Can you make this const?
- Niklas Nielsen
On Jan. 21, 2014, 11:34 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2014, 11:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/container/cgroups_launcher.hpp PRE-CREATION
> src/slave/container/cgroups_launcher.cpp PRE-CREATION
> src/slave/container/launcher.hpp PRE-CREATION
> src/slave/container/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review32432
-----------------------------------------------------------
Ship it!
Looks great!
src/Makefile.am
<https://reviews.apache.org/r/16149/#comment61260>
Alpha order? Keep linux/* together, or does this need to be this way?
src/slave/container/cgroups_launcher.hpp
<https://reviews.apache.org/r/16149/#comment61758>
__CGROUPS_LAUNCHER_HPP__
src/slave/container/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment61764>
Why not assign "path::join(flags.cgroups_root, "test")" to a variable, if it's going to be used 3 times in this function?
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment61759>
__LAUNCHER_HPP__
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment61762>
I usually like to give collections plural names, so a list<RunState> would be named "states"
src/slave/container/launcher.hpp
<https://reviews.apache.org/r/16149/#comment61760>
"Launcher suitable for" (remove the extra "for")
src/slave/container/launcher.cpp
<https://reviews.apache.org/r/16149/#comment61763>
Do you want to include messages for any of these ErrnoError()s?
- Adam B
On Jan. 21, 2014, 11:34 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2014, 11:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/container/cgroups_launcher.hpp PRE-CREATION
> src/slave/container/cgroups_launcher.cpp PRE-CREATION
> src/slave/container/launcher.hpp PRE-CREATION
> src/slave/container/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
> On Jan. 28, 2014, 7:13 p.m., Vinod Kone wrote:
> > src/slave/containerizer/cgroups_launcher.cpp, lines 162-165
> > <https://reviews.apache.org/r/16149/diff/7/?file=451161#file451161line162>
> >
> > Why is this an error?
> >
> > If the cgroup was removed but the slave died before it got the signal from the containerizer, wouldn't we be here?
> >
> > I think the right thing to do here is to just skip it.
Good point; I've amended it to skip this case. I also commented how the containerizer will react (monitor pid -> destroy)
> On Jan. 28, 2014, 7:13 p.m., Vinod Kone wrote:
> > src/slave/containerizer/cgroups_launcher.cpp, line 197
> > <https://reviews.apache.org/r/16149/diff/7/?file=451161#file451161line197>
> >
> > How is it possible that a cgroup for this container already exists? If it does, shouldn't we return an error?
Launchers support forking multiple processes for a container. This will be used in the near future. PosixLauncher will put each new process into the same session, CgroupsLauncher will put each new process into the same freezer cgroup.
> On Jan. 28, 2014, 7:13 p.m., Vinod Kone wrote:
> > src/slave/containerizer/cgroups_launcher.cpp, line 207
> > <https://reviews.apache.org/r/16149/diff/7/?file=451161#file451161line207>
> >
> > s/it's/its/
it has -> it's
> On Jan. 28, 2014, 7:13 p.m., Vinod Kone wrote:
> > src/slave/containerizer/cgroups_launcher.cpp, line 264
> > <https://reviews.apache.org/r/16149/diff/7/?file=451161#file451161line264>
> >
> > The older semantics was to also do a setsid() here. Why the regression?
Assumption is that sessions are not necessary when the processes are contained in a cgroup.
> On Jan. 28, 2014, 7:13 p.m., Vinod Kone wrote:
> > src/slave/containerizer/launcher.cpp, lines 45-47
> > <https://reviews.apache.org/r/16149/diff/7/?file=451163#file451163line45>
> >
> > ditto. see the comment in cgroupslauncher::recover().
This case is a little different: MesosContainerizer only tries to recover containers with pids and skips those without so it is an error to try to recover a container without a pid. In the CgroupsLauncher it was possible that there was a pid but the cgroup had been cleaned up so there it was correct to change to skip.
> On Jan. 28, 2014, 7:13 p.m., Vinod Kone wrote:
> > src/slave/containerizer/launcher.cpp, lines 49-51
> > <https://reviews.apache.org/r/16149/diff/7/?file=451163#file451163line49>
> >
> > How is this possible? If there is pid wrapping? If yes, please add a comment.
Technically it could happen with just the right timing of executor exiting and new executor with same pid and slave dying after new but before hearing about old. Regardless, the launcher cannot support this so it's considered an error.
comments added.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review33012
-----------------------------------------------------------
On Jan. 27, 2014, 3:02 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2014, 3:02 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
> src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
> src/slave/containerizer/launcher.hpp PRE-CREATION
> src/slave/containerizer/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review33012
-----------------------------------------------------------
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment62140>
s/subsystem/subsystem " << subsystem <</
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment62142>
Why is this an error?
If the cgroup was removed but the slave died before it got the signal from the containerizer, wouldn't we be here?
I think the right thing to do here is to just skip it.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment62151>
s/fork process/create freezer cgroup/ ?
also please include exists.error() in the Error().
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment62152>
How is it possible that a cgroup for this container already exists? If it does, shouldn't we return an error?
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment62154>
s/it's/its/
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment62162>
The older semantics was to also do a setsid() here. Why the regression?
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment62155>
Include destroyed.failure() in Failure() please.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment62156>
ditto. see the comment in cgroupslauncher::recover().
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment62157>
How is this possible? If there is pid wrapping? If yes, please add a comment.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment62158>
s/Addition/Additional/
- Vinod Kone
On Jan. 27, 2014, 3:02 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2014, 3:02 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
> src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
> src/slave/containerizer/launcher.hpp PRE-CREATION
> src/slave/containerizer/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
> On Jan. 29, 2014, 5:52 p.m., Niklas Nielsen wrote:
> > src/slave/containerizer/cgroups_launcher.cpp, line 259
> > <https://reviews.apache.org/r/16149/diff/7/?file=451161#file451161line259>
> >
> > GCC 4.8.2 fails here; error: ignoring return value of ‘ssize_t write(int, const void*, size_t)’, declared with attribute warn_unused_result [-Werror=unused-result]
> >
> > Same thing applies for mesos_containerizer.cpp:231
Sorry about that, it wasn't complaining on my 4.2.1 so I missed it but you're right. Changed to ignore the return values.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review33125
-----------------------------------------------------------
On Jan. 27, 2014, 3:02 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2014, 3:02 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
> src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
> src/slave/containerizer/launcher.hpp PRE-CREATION
> src/slave/containerizer/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review33125
-----------------------------------------------------------
Hey Ian,
Took your latest diff for a spin and ran into a compile error on the way. It should be a simple fix.
BTW Are there diffs on the way? The patch set needs to be rebased.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment62406>
GCC 4.8.2 fails here; error: ignoring return value of ‘ssize_t write(int, const void*, size_t)’, declared with attribute warn_unused_result [-Werror=unused-result]
Same thing applies for mesos_containerizer.cpp:231
- Niklas Nielsen
On Jan. 26, 2014, 7:02 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2014, 7:02 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
> src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
> src/slave/containerizer/launcher.hpp PRE-CREATION
> src/slave/containerizer/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Feb. 12, 2014, 12:18 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Adressed Ben's comments.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/linux/cgroups.hpp bf0d17354aa8b601014f63c3b42141c74855b959
src/linux/cgroups.cpp 19ab1f348191ab0315271477b206aa8c6456fd5a
src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
src/slave/containerizer/launcher.hpp PRE-CREATION
src/slave/containerizer/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review33918
-----------------------------------------------------------
Ship it!
src/slave/containerizer/cgroups_launcher.hpp
<https://reviews.apache.org/r/16149/#comment63678>
Kill the newline.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63683>
Why is this an instance field?
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63682>
I know the original CgroupsIsolator had lots of CHECKs and EXITs but we should really start transitioning them over to Errors.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63684>
Same comments in other reviews as to why not returning Errors.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63685>
Ignore EINTR?
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63687>
I'm confused how you guarantee that this process has the same session as 'pgid' as required by 'setpgid'? I'm guessing this code is not exercised anywhere (including tests) since we don't fork extra processes in a container.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63686>
I don't think 'perror' is signal safe but I also think that's okay.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63688>
Ignore EINTR?
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63689>
Why '_exit' here but 'abort' above? I noticed this in the last review too.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63690>
This feels like a bit of a hack. Why not 'return -1' or even an 'Error("unreachable")' or use 'UNREACHABLE()' rather than give the illusion that 'inChild' is actually supposed to return anything? If you did want 'inChild' to return something you should really be doing 'exit(inChild())'.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63691>
&
src/slave/containerizer/launcher.hpp
<https://reviews.apache.org/r/16149/#comment63679>
Above the stout headers please.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63692>
See comments above re: session.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63680>
See comments above.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63681>
Space after 'container'.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63693>
I don't understand your comment. Do you mean killtree will fail if the "root" process isn't found? The code below seems a bit strange: if there is only the root pid than os::pids will return a non-empty set and we'll just execute killtree as normal so it's unclear what doing os::pids first bought us. If the root process no longer exists but some other processes from the session still exist then calling os::pids will return a non-empty set but then calling killtree will fail and we'll leave some processes running. It seems like we want to try and do killtree and maybe also try and kill all processes in the process group and session?
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63694>
What do you mean by parent here?
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63695>
We should not be able to reap any granchildren, only each 'tree.process.pid' in the returned 'trees' from 'killtree'.
- Benjamin Hindman
On Feb. 7, 2014, 7:09 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Feb. 7, 2014, 7:09 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
> src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
> src/slave/containerizer/launcher.hpp PRE-CREATION
> src/slave/containerizer/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Feb. 7, 2014, 7:09 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Minor tweaks from Vinod's comments.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
src/slave/containerizer/launcher.hpp PRE-CREATION
src/slave/containerizer/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review33899
-----------------------------------------------------------
Ship it!
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63615>
I thought we were going for INFO here because it's expected case.
Also, you might want to pull down the comment inside the if loop.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63616>
Maybe include the pid and container id?
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63617>
ditto.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63619>
Much cleaner!
- Vinod Kone
On Feb. 7, 2014, 3:36 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Feb. 7, 2014, 3:36 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
> src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
> src/slave/containerizer/launcher.hpp PRE-CREATION
> src/slave/containerizer/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Feb. 7, 2014, 3:36 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Addressed Vinod's comments.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
src/slave/containerizer/launcher.hpp PRE-CREATION
src/slave/containerizer/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
> On Feb. 6, 2014, 11:28 p.m., Vinod Kone wrote:
> > src/slave/containerizer/cgroups_launcher.cpp, line 298
> > <https://reviews.apache.org/r/16149/diff/7-9/?file=451161#file451161line298>
> >
> > make it consistent with subprocess implementation.
crept into a later review.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review33857
-----------------------------------------------------------
On Feb. 6, 2014, 5:35 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2014, 5:35 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
> src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
> src/slave/containerizer/launcher.hpp PRE-CREATION
> src/slave/containerizer/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review33857
-----------------------------------------------------------
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63494>
A better error message?
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63496>
maybe add why its needed.
src/slave/containerizer/cgroups_launcher.cpp
<https://reviews.apache.org/r/16149/#comment63497>
make it consistent with subprocess implementation.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63500>
ditto.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63483>
This should be containsValue()?
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63501>
new line.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63503>
same line.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63504>
s/;//
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63520>
Rewrite this as discussed offline.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63518>
move the dequeue inside the loop below?
traverse the entire tree first before moving on to the next tree. consider having a kill(ProcessTree) function and doing to recursively.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63506>
new line.
src/slave/containerizer/launcher.cpp
<https://reviews.apache.org/r/16149/#comment63507>
include container id.
- Vinod Kone
On Feb. 6, 2014, 5:35 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2014, 5:35 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
> src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
> src/slave/containerizer/launcher.hpp PRE-CREATION
> src/slave/containerizer/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Feb. 6, 2014, 5:35 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Added a PosixDestroyer which kills all processes launched by a PosixLauncher.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
src/slave/containerizer/launcher.hpp PRE-CREATION
src/slave/containerizer/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Feb. 1, 2014, 4:02 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Rebased, addressed comments and fixes around setsid for cgroups_launcher.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
src/slave/containerizer/launcher.hpp PRE-CREATION
src/slave/containerizer/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Jan. 27, 2014, 3:02 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Addressed review comments.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
src/slave/containerizer/cgroups_launcher.hpp PRE-CREATION
src/slave/containerizer/cgroups_launcher.cpp PRE-CREATION
src/slave/containerizer/launcher.hpp PRE-CREATION
src/slave/containerizer/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers (part 2)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Jan. 21, 2014, 7:34 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Summary (updated)
-----------------
Containerizer - launchers (part 2)
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs
-----
src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
src/slave/container/cgroups_launcher.hpp PRE-CREATION
src/slave/container/cgroups_launcher.cpp PRE-CREATION
src/slave/container/launcher.hpp PRE-CREATION
src/slave/container/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Jan. 17, 2014, 1:36 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
src/slave/container/cgroups_launcher.hpp PRE-CREATION
src/slave/container/cgroups_launcher.cpp PRE-CREATION
src/slave/container/launcher.hpp PRE-CREATION
src/slave/container/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Jan. 17, 2014, 1:33 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Refactored launchers to just do fork() (or clone()) in the containerized context and then to kill all processes in the context.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
src/slave/container/cgroups_launcher.hpp PRE-CREATION
src/slave/container/cgroups_launcher.cpp PRE-CREATION
src/slave/container/launcher.hpp PRE-CREATION
src/slave/container/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/#review31697
-----------------------------------------------------------
This patch, #16150 and #16147 must be applied in a specific order, right? Can we capture this in the "depends on"/"blocks" fields?
- Niklas Nielsen
On Dec. 20, 2013, 10:55 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16149/
> -----------------------------------------------------------
>
> (Updated Dec. 20, 2013, 10:55 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Launcher interface and MesosLauncher to support MesosContainerizers.
>
> Launchers handle the lifecycle of the executor process (and descendants).
>
>
> Diffs
> -----
>
> src/Makefile.am 42dafbcd6e2d4f67d2705c5a2bea6ec6a0f4fcc1
> src/slave/container/launcher.hpp PRE-CREATION
> src/slave/container/launcher.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16149/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16149: Containerizer - launchers
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Dec. 21, 2013, 6:55 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs
-----
src/Makefile.am 42dafbcd6e2d4f67d2705c5a2bea6ec6a0f4fcc1
src/slave/container/launcher.hpp PRE-CREATION
src/slave/container/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Dec. 21, 2013, 6:54 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Fixed compilation on linux. Addressed Niklas' comments.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am 42dafbcd6e2d4f67d2705c5a2bea6ec6a0f4fcc1
src/slave/container/launcher.hpp PRE-CREATION
src/slave/container/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Dec. 19, 2013, 8:01 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Addressed comments.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/Makefile.am 42dafbcd6e2d4f67d2705c5a2bea6ec6a0f4fcc1
src/slave/container/launcher.hpp PRE-CREATION
src/slave/container/launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16149: Containerizer - launchers
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16149/
-----------------------------------------------------------
(Updated Dec. 11, 2013, 4:06 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Rebased master.
Repository: mesos-git
Description
-------
Launcher interface and MesosLauncher to support MesosContainerizers.
Launchers handle the lifecycle of the executor process (and descendants).
Diffs (updated)
-----
src/slave/container/launcher.hpp PRE-CREATION
src/slave/container/launcher.cpp PRE-CREATION
src/slave/container/mesos_launcher.hpp PRE-CREATION
src/slave/container/mesos_launcher.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/16149/diff/
Testing
-------
Thanks,
Ian Downes