You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2014/09/05 05:10:41 UTC
Review Request 25372: Made the GarbageCollector injectable into the
Slave.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/
-----------------------------------------------------------
Review request for mesos and Vinod Kone.
Bugs: MESOS-1715
https://issues.apache.org/jira/browse/MESOS-1715
Repository: mesos-git
Description
-------
Need to inject the GarbageCollector for testing MESOS-1715.
I also cleaned up the testing cluster code.
Diffs
-----
src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
Diff: https://reviews.apache.org/r/25372/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/#review52775
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Sept. 9, 2014, 12:43 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25372/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 12:43 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1715
> https://issues.apache.org/jira/browse/MESOS-1715
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> I also cleaned up the testing cluster code.
>
>
> Diffs
> -----
>
> src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
> src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
> src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
> src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
> src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
> src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
> src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
> src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
> src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
>
> Diff: https://reviews.apache.org/r/25372/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/
-----------------------------------------------------------
(Updated Sept. 9, 2014, 12:43 a.m.)
Review request for mesos and Vinod Kone.
Changes
-------
Updated description to reflect that we don't strictly need the GC injection for now.
Bugs: MESOS-1715
https://issues.apache.org/jira/browse/MESOS-1715
Repository: mesos-git
Description (updated)
-------
I also cleaned up the testing cluster code.
Diffs
-----
src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
Diff: https://reviews.apache.org/r/25372/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/
-----------------------------------------------------------
(Updated Sept. 9, 2014, 12:42 a.m.)
Review request for mesos and Vinod Kone.
Changes
-------
Vinod's review.
Bugs: MESOS-1715
https://issues.apache.org/jira/browse/MESOS-1715
Repository: mesos-git
Description
-------
Need to inject the GarbageCollector for testing MESOS-1715.
I also cleaned up the testing cluster code.
Diffs (updated)
-----
src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
Diff: https://reviews.apache.org/r/25372/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/#review52612
-----------------------------------------------------------
src/tests/mesos.hpp
<https://reviews.apache.org/r/25372/#comment91462>
Actually, you don't need "testing::An" here AFAICT. It is needed when you have overloaded Mock methods (like Authorizer::authorize()).
- Vinod Kone
On Sept. 5, 2014, 3:10 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25372/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2014, 3:10 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1715
> https://issues.apache.org/jira/browse/MESOS-1715
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Need to inject the GarbageCollector for testing MESOS-1715.
>
> I also cleaned up the testing cluster code.
>
>
> Diffs
> -----
>
> src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
> src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
> src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
> src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
> src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
> src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
> src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
> src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
> src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
>
> Diff: https://reviews.apache.org/r/25372/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Ben Mahler <be...@gmail.com>.
> On Sept. 8, 2014, 5:42 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, lines 155-164
> > <https://reviews.apache.org/r/25372/diff/1/?file=679951#file679951line155>
> >
> > Similar to Slave do you think these should be Owned?
Good call, cleaned these up as well.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/#review52518
-----------------------------------------------------------
On Sept. 5, 2014, 3:10 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25372/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2014, 3:10 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1715
> https://issues.apache.org/jira/browse/MESOS-1715
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Need to inject the GarbageCollector for testing MESOS-1715.
>
> I also cleaned up the testing cluster code.
>
>
> Diffs
> -----
>
> src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
> src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
> src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
> src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
> src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
> src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
> src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
> src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
> src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
>
> Diff: https://reviews.apache.org/r/25372/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Ben Mahler <be...@gmail.com>.
> On Sept. 8, 2014, 5:42 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, line 597
> > <https://reviews.apache.org/r/25372/diff/1/?file=679951#file679951line597>
> >
> > Since you are cleaning up this code, I don't think this variable is necessary anymore? Actually I don't know why it was used in the first place, because the other injected "things" did not need a boolean to keep track of their injection.
>
> Vinod Kone wrote:
> dropped without comment?
Whoops, looks like my comment was lost for some reason. This one is needed because we need a handle to the containerizer (for shutting down containers), whereas we don't need to keep handles to the other injected components.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/#review52518
-----------------------------------------------------------
On Sept. 9, 2014, 12:43 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25372/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 12:43 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1715
> https://issues.apache.org/jira/browse/MESOS-1715
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> I also cleaned up the testing cluster code.
>
>
> Diffs
> -----
>
> src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
> src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
> src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
> src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
> src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
> src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
> src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
> src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
> src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
>
> Diff: https://reviews.apache.org/r/25372/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Vinod Kone <vi...@gmail.com>.
> On Sept. 8, 2014, 5:42 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, line 597
> > <https://reviews.apache.org/r/25372/diff/1/?file=679951#file679951line597>
> >
> > Since you are cleaning up this code, I don't think this variable is necessary anymore? Actually I don't know why it was used in the first place, because the other injected "things" did not need a boolean to keep track of their injection.
>
> Vinod Kone wrote:
> dropped without comment?
>
> Ben Mahler wrote:
> Whoops, looks like my comment was lost for some reason. This one is needed because we need a handle to the containerizer (for shutting down containers), whereas we don't need to keep handles to the other injected components.
aha. that makes sense.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/#review52518
-----------------------------------------------------------
On Sept. 9, 2014, 12:43 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25372/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 12:43 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1715
> https://issues.apache.org/jira/browse/MESOS-1715
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> I also cleaned up the testing cluster code.
>
>
> Diffs
> -----
>
> src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
> src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
> src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
> src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
> src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
> src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
> src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
> src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
> src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
>
> Diff: https://reviews.apache.org/r/25372/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Vinod Kone <vi...@gmail.com>.
> On Sept. 8, 2014, 5:42 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, line 597
> > <https://reviews.apache.org/r/25372/diff/1/?file=679951#file679951line597>
> >
> > Since you are cleaning up this code, I don't think this variable is necessary anymore? Actually I don't know why it was used in the first place, because the other injected "things" did not need a boolean to keep track of their injection.
dropped without comment?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/#review52518
-----------------------------------------------------------
On Sept. 9, 2014, 12:43 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25372/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 12:43 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1715
> https://issues.apache.org/jira/browse/MESOS-1715
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> I also cleaned up the testing cluster code.
>
>
> Diffs
> -----
>
> src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
> src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
> src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
> src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
> src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
> src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
> src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
> src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
> src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
>
> Diff: https://reviews.apache.org/r/25372/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 25372: Made the GarbageCollector injectable into
the Slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25372/#review52518
-----------------------------------------------------------
src/local/local.cpp
<https://reviews.apache.org/r/25372/#comment91314>
Not sure why we are sharing the "gc" among the slaves. Actually, same with "detector" and "files". I guess it's thread safe because they are simple wrappers that dispatch to libprocesses.
src/tests/cluster.hpp
<https://reviews.apache.org/r/25372/#comment91315>
I think the old comment for this method was more informative and useful. Why not just use that?
src/tests/cluster.hpp
<https://reviews.apache.org/r/25372/#comment91458>
Similar to Slave do you think these should be Owned?
src/tests/cluster.hpp
<https://reviews.apache.org/r/25372/#comment91317>
Ditto. A blurb about the default containerizer etc and the lifetime would be useful info to comment.
src/tests/cluster.hpp
<https://reviews.apache.org/r/25372/#comment91457>
Since you are cleaning up this code, I don't think this variable is necessary anymore? Actually I don't know why it was used in the first place, because the other injected "things" did not need a boolean to keep track of their injection.
src/tests/fault_tolerance_tests.cpp
<https://reviews.apache.org/r/25372/#comment91459>
Who is using "An" in this file?
- Vinod Kone
On Sept. 5, 2014, 3:10 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25372/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2014, 3:10 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1715
> https://issues.apache.org/jira/browse/MESOS-1715
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Need to inject the GarbageCollector for testing MESOS-1715.
>
> I also cleaned up the testing cluster code.
>
>
> Diffs
> -----
>
> src/local/local.cpp 9db4dd18c1aefca24cfa66d762994ac59ed2ed3d
> src/slave/gc.hpp 8f9398b1d8bf4b2615e632accade4cfa79d7b62d
> src/slave/main.cpp 319316f8f95cddbfa4b5d0656f4377e3b8d6ade1
> src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
> src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/fault_tolerance_tests.cpp b0e74b94db8a68cd676569d42b2ccd120f12b9d3
> src/tests/master_authorization_tests.cpp b9aa7bf4f53e414d84f8cf4e020a645db8e5d855
> src/tests/mesos.hpp b31c347299707cba242619c3dc6915f295bee9cb
> src/tests/mesos.cpp 0f759a70b457561bc878fa62b819142a064d5ea4
> src/tests/reconciliation_tests.cpp 8c66659982ceff37572a89628221c40be1c59406
>
> Diff: https://reviews.apache.org/r/25372/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>