Re: Review Request 25372: Made the GarbageCollector injectable into the Slave.

2014-09-09 Thread Vinod Kone 


> On Sept. 8, 2014, 5:42 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, line 597
> > 
> >
> > 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.

2014-09-09 Thread Vinod Kone 

---
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.

2014-09-09 Thread Ben Mahler 


> On Sept. 8, 2014, 5:42 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, line 597
> > 
> >
> > 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.

2014-09-08 Thread Vinod Kone 


> On Sept. 8, 2014, 5:42 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, line 597
> > 
> >
> > 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.

2014-09-08 Thread Ben Mahler 

---
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.

2014-09-08 Thread Ben Mahler 

---
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.

2014-09-08 Thread Ben Mahler 


> On Sept. 8, 2014, 5:42 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, lines 155-164
> > 
> >
> > 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.

2014-09-08 Thread Vinod Kone 

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



src/tests/mesos.hpp


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.

2014-09-08 Thread Vinod Kone 

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



src/local/local.cpp


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


I think the old comment for this method was more informative and useful. 
Why not just use that?



src/tests/cluster.hpp


Similar to Slave do you think these should be Owned?



src/tests/cluster.hpp


Ditto. A blurb about the default containerizer etc and the lifetime would 
be useful info to comment.



src/tests/cluster.hpp


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


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
> 
>

Review Request 25372: Made the GarbageCollector injectable into the Slave.

2014-09-04 Thread Ben Mahler 

---
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