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:26:56 UTC
Review Request 16150: Containerizer - isolators
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Repository: mesos-git
Description
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs
-----
src/slave/container/isolator.hpp PRE-CREATION
src/slave/container/isolator.cpp PRE-CREATION
src/slave/container/isolators/cpu/posix.hpp PRE-CREATION
src/slave/container/isolators/cpu/posix.cpp PRE-CREATION
src/slave/container/isolators/mem/posix.hpp PRE-CREATION
src/slave/container/isolators/mem/posix.cpp PRE-CREATION
src/slave/container/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16150: Containerizer - isolators
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review30238
-----------------------------------------------------------
benh and I went over this today, this is a partial review.
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment57886>
Rather than using an Option with None() as a sentinel, can this take the full state and perform any cleanup as part of the single call to recover()?
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment57890>
Why does this return a string script? Couldn't the script be executed by the isolator during the isolate() call?
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment57887>
ExecutorInfo contains resources, why are resources being passed separately here?
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment57888>
Cleanup? Destroy seems to imply that this is destroying the underlying processes.
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment57889>
In general we place the API (in this case: Isolator) above the libprocess Process.
src/slave/container/isolator.cpp
<https://reviews.apache.org/r/16150/#comment57891>
CHECK_NOTNULL(process.get())
src/slave/container/isolators/cpu/posix.cpp
<https://reviews.apache.org/r/16150/#comment57897>
Can this use pstree instead? (pstree didn't exist when I first implemented posix usage collection).
There seems to be a lot of redundant code in order to split apart the cpu and memory usage() reporting for posix. Is there a major gain from doing this? I would prefer to see less code by avoiding the posix inheritance.
src/slave/container/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment57895>
Please use virtual to signify that this is a virtual function that you are overloading, here and elsewhere.
src/slave/container/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment57892>
Can this return a Failure instead?
src/slave/container/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment57893>
return Future<Nothing>();
src/slave/container/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment57896>
You don't need this, unless you want this to show that it is unimplemented?
src/slave/container/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment57894>
No need for a promise, see above. Also, the downside of a promise is that if a single person discards the future then all clients of watch() will get the discarded future, whereas with returning the Future each client has a different Future.
- Ben Mahler
On Dec. 11, 2013, 4:09 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16150/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2013, 4:09 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/slave/container/isolator.hpp PRE-CREATION
> src/slave/container/isolator.cpp PRE-CREATION
> src/slave/container/isolators/cpu/posix.hpp PRE-CREATION
> src/slave/container/isolators/cpu/posix.cpp PRE-CREATION
> src/slave/container/isolators/mem/posix.hpp PRE-CREATION
> src/slave/container/isolators/mem/posix.cpp PRE-CREATION
> src/slave/container/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review30283
-----------------------------------------------------------
src/slave/container/isolators/cpu/posix.hpp
<https://reviews.apache.org/r/16150/#comment57972>
I'm not convinced that we need subclasses of PosixIsolatorProcess. Any reason we can't do both CPU and Mem together for now? I don't see any problem since we'll be merging the ResourceStatistics anyway. It's less code this way too. At the very least, the shared code in each usage should be factored out (and see the comment about trying to use os::pstree).
- Benjamin Hindman
On Dec. 11, 2013, 4:09 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16150/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2013, 4:09 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/slave/container/isolator.hpp PRE-CREATION
> src/slave/container/isolator.cpp PRE-CREATION
> src/slave/container/isolators/cpu/posix.hpp PRE-CREATION
> src/slave/container/isolators/cpu/posix.cpp PRE-CREATION
> src/slave/container/isolators/mem/posix.hpp PRE-CREATION
> src/slave/container/isolators/mem/posix.cpp PRE-CREATION
> src/slave/container/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review30693
-----------------------------------------------------------
Same as containerizer / launchers review - will follow up with style review.
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment58796>
~IsolatorProcess() needs an implementation.
- Niklas Nielsen
On Dec. 19, 2013, 8 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16150/
> -----------------------------------------------------------
>
> (Updated Dec. 19, 2013, 8 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am 42dafbcd6e2d4f67d2705c5a2bea6ec6a0f4fcc1
> src/slave/container/isolator.hpp PRE-CREATION
> src/slave/container/isolator.cpp PRE-CREATION
> src/slave/container/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Ian Downes <ia...@gmail.com>.
> On Jan. 26, 2014, 8:08 a.m., Adam B wrote:
> > src/slave/container/isolator.cpp, line 99
> > <https://reviews.apache.org/r/16150/diff/4/?file=425490#file425490line99>
> >
> > Needs newline at end of file
Not needed for cpp file, and not done by other files in codebase.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review32792
-----------------------------------------------------------
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/16150/
> -----------------------------------------------------------
>
> (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
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/containerizer/isolator.hpp PRE-CREATION
> src/slave/containerizer/isolator.cpp PRE-CREATION
> src/slave/containerizer/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review32792
-----------------------------------------------------------
Ship it!
Looks great!
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment61765>
"states" plural
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment61766>
Should the CommandInfo comment go with the isolate() method that returns the CommandInfo?
src/slave/container/isolator.cpp
<https://reviews.apache.org/r/16150/#comment61767>
Needs newline at end of file
src/slave/container/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment61769>
Neither parameter is used here. Could you comment why update does nothing?
- 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/16150/
> -----------------------------------------------------------
>
> (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
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/container/isolator.hpp PRE-CREATION
> src/slave/container/isolator.cpp PRE-CREATION
> src/slave/container/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Ian Downes <ia...@gmail.com>.
> On Jan. 28, 2014, 7:42 p.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/posix.hpp, line 36
> > <https://reviews.apache.org/r/16150/diff/5/?file=451176#file451176line36>
> >
> > I'm assuming you don't people to directly use PosixIsolatorProcess but rather its subclasses?
> >
> > If yes, should we make its constructor(s) private?
It hasn't implemented Isolator::usage() so it's still abstract. Is that sufficient or should I make the constructor private too?
> On Jan. 28, 2014, 7:42 p.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/posix.hpp, lines 52-54
> > <https://reviews.apache.org/r/16150/diff/5/?file=451176#file451176line52>
> >
> > As mentioned in earlier reviews, this shouldn't be an error?
MesosContainerizer only tries to recover containers from the isolators and launchers if they have pids.
> On Jan. 28, 2014, 7:42 p.m., Vinod Kone wrote:
> > src/slave/containerizer/isolators/posix.hpp, line 140
> > <https://reviews.apache.org/r/16150/diff/5/?file=451176#file451176line140>
> >
> > What if the pid exits by this point? Should it be a Failure or return empty resource stats?
Yes, should just return empty stats.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review33022
-----------------------------------------------------------
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/16150/
> -----------------------------------------------------------
>
> (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
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/containerizer/isolator.hpp PRE-CREATION
> src/slave/containerizer/isolator.cpp PRE-CREATION
> src/slave/containerizer/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review33022
-----------------------------------------------------------
src/slave/containerizer/isolator.hpp
<https://reviews.apache.org/r/16150/#comment62168>
const?
src/slave/containerizer/isolator.hpp
<https://reviews.apache.org/r/16150/#comment62169>
const ?
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment62174>
I'm assuming you don't people to directly use PosixIsolatorProcess but rather its subclasses?
If yes, should we make its constructor(s) private?
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment62171>
As mentioned in earlier reviews, this shouldn't be an error?
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment62172>
How is this possible? Comment?
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment62182>
What if the pid exits by this point? Should it be a Failure or return empty resource stats?
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment62183>
ditto.
- 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/16150/
> -----------------------------------------------------------
>
> (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
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/containerizer/isolator.hpp PRE-CREATION
> src/slave/containerizer/isolator.cpp PRE-CREATION
> src/slave/containerizer/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review33864
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Feb. 1, 2014, 4:03 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16150/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2014, 4:03 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/slave/containerizer/isolator.hpp PRE-CREATION
> src/slave/containerizer/isolator.cpp PRE-CREATION
> src/slave/containerizer/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
(Updated Feb. 12, 2014, 12:20 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
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/slave/containerizer/isolator.hpp PRE-CREATION
src/slave/containerizer/isolator.cpp PRE-CREATION
src/slave/containerizer/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review33919
-----------------------------------------------------------
Ship it!
src/slave/containerizer/isolator.hpp
<https://reviews.apache.org/r/16150/#comment63700>
Please wrap everything at 80 columns!
src/slave/containerizer/isolator.hpp
<https://reviews.apache.org/r/16150/#comment63697>
Can kill this newline.
src/slave/containerizer/isolator.cpp
<https://reviews.apache.org/r/16150/#comment63698>
FYI, you can also do 'process::spawn(CHECK_NOTNULL(process.get()))'.
src/slave/containerizer/isolator.cpp
<https://reviews.apache.org/r/16150/#comment63699>
Please wrap everything in here at 80 columns.
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment63703>
Wrap at 80 columns pretty pretty please!
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment63701>
The wrapping is usually:
process::Owned<process::Promise<Limitation> > promise(
new process::Promise<Limitation>());
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment63702>
The wrapping is usually:
process::Owned<process::Promise<Limitation> > promise(
new process::Promise<Limitation>());
src/slave/containerizer/isolators/posix.hpp
<https://reviews.apache.org/r/16150/#comment63704>
You can kill the newline.
- Benjamin Hindman
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/16150/
> -----------------------------------------------------------
>
> (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
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/slave/containerizer/isolator.hpp PRE-CREATION
> src/slave/containerizer/isolator.cpp PRE-CREATION
> src/slave/containerizer/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
(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
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/slave/containerizer/isolator.hpp PRE-CREATION
src/slave/containerizer/isolator.cpp PRE-CREATION
src/slave/containerizer/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
(Updated Feb. 1, 2014, 4:03 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Rebased and addressed comments.
Repository: mesos-git
Description
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/slave/containerizer/isolator.hpp PRE-CREATION
src/slave/containerizer/isolator.cpp PRE-CREATION
src/slave/containerizer/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
(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
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs (updated)
-----
src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
src/slave/containerizer/isolator.hpp PRE-CREATION
src/slave/containerizer/isolator.cpp PRE-CREATION
src/slave/containerizer/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review32799
-----------------------------------------------------------
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment61777>
clang emitted a 'hidden virtual overload' error - that's way I mentioned it :)
- 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/16150/
> -----------------------------------------------------------
>
> (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
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/container/isolator.hpp PRE-CREATION
> src/slave/container/isolator.cpp PRE-CREATION
> src/slave/container/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Ian Downes <ia...@gmail.com>.
> On Jan. 23, 2014, 6:39 p.m., Niklas Nielsen wrote:
> > src/slave/container/isolator.hpp, line 108
> > <https://reviews.apache.org/r/16150/diff/4/?file=425489#file425489line108>
> >
> > This initialize overshadows initialize in Process. Can use resolve this with 'using process::ProcessBase::initialize;'?
This won't overshadow process::ProcessBase::initialize()?
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review32553
-----------------------------------------------------------
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/16150/
> -----------------------------------------------------------
>
> (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
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/container/isolator.hpp PRE-CREATION
> src/slave/container/isolator.cpp PRE-CREATION
> src/slave/container/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/#review32553
-----------------------------------------------------------
Ship it!
src/Makefile.am
<https://reviews.apache.org/r/16150/#comment61433>
Align '\''s
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment61434>
Can you comment this call too?
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment61435>
This one.
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment61436>
This one.
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment61437>
And this one.
src/slave/container/isolator.hpp
<https://reviews.apache.org/r/16150/#comment61438>
This initialize overshadows initialize in Process. Can use resolve this with 'using process::ProcessBase::initialize;'?
- 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/16150/
> -----------------------------------------------------------
>
> (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
> -------
>
> Isolators perform isolator for the MesosContainerizer
>
> Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
>
>
> Diffs
> -----
>
> src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
> src/slave/container/isolator.hpp PRE-CREATION
> src/slave/container/isolator.cpp PRE-CREATION
> src/slave/container/isolators/posix.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16150/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 16150: Containerizer - isolators (part 3)
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
(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 - isolators (part 3)
Repository: mesos-git
Description
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs
-----
src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
src/slave/container/isolator.hpp PRE-CREATION
src/slave/container/isolator.cpp PRE-CREATION
src/slave/container/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16150: Containerizer - isolators
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
(Updated Jan. 17, 2014, 1:36 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Niklas Nielsen, samya, and Jason Dusek.
Changes
-------
Rebased + fixes.
Repository: mesos-git
Description
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs (updated)
-----
src/Makefile.am cf0c8c66e8dd21c2f5a2da22e5d5adb056353454
src/slave/container/isolator.hpp PRE-CREATION
src/slave/container/isolator.cpp PRE-CREATION
src/slave/container/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16150: Containerizer - isolators
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
(Updated Dec. 19, 2013, 8 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
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs (updated)
-----
src/Makefile.am 42dafbcd6e2d4f67d2705c5a2bea6ec6a0f4fcc1
src/slave/container/isolator.hpp PRE-CREATION
src/slave/container/isolator.cpp PRE-CREATION
src/slave/container/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 16150: Containerizer - isolators
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16150/
-----------------------------------------------------------
(Updated Dec. 11, 2013, 4:09 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
-------
Isolators perform isolator for the MesosContainerizer
Isolator interface and implementations of Posix CPU and Mem isolators (no isolation, just usage())
Diffs (updated)
-----
src/slave/container/isolator.hpp PRE-CREATION
src/slave/container/isolator.cpp PRE-CREATION
src/slave/container/isolators/cpu/posix.hpp PRE-CREATION
src/slave/container/isolators/cpu/posix.cpp PRE-CREATION
src/slave/container/isolators/mem/posix.hpp PRE-CREATION
src/slave/container/isolators/mem/posix.cpp PRE-CREATION
src/slave/container/isolators/posix.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/16150/diff/
Testing
-------
Thanks,
Ian Downes