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 2013/03/19 22:02:24 UTC
Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
See above. This was needed in order to write a fix for MESOS-396.
This addresses bug MESOS-396.
https://issues.apache.org/jira/browse/MESOS-396
Diffs
-----
src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
src/tests/gc_tests.cpp a9a7a439702ab87d5384c2d1b8afc0b17dab4e62
Diff: https://reviews.apache.org/r/10028/diff/
Testing
-------
Added missing and new unit tests.
Also ran the tests with 300 iterations.
Thanks,
Ben Mahler
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
> On March 21, 2013, 12:37 a.m., Ben Mahler wrote:
> > src/slave/gc.cpp, line 99
> > <https://reviews.apache.org/r/10028/diff/1/?file=272103#file272103line99>
> >
> > Move these delete operations into a PathInfo destructor, then these loops can be const& again.
Doing this caused some strange libprocess issues to arise (sometimes deadlocking, sometimes an assertion failure).
At first glance it was not obvious what the issue was, so let me know if you want to debug this together!
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18164
-----------------------------------------------------------
On March 21, 2013, 11:29 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 21, 2013, 11:29 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18164
-----------------------------------------------------------
src/slave/gc.cpp
<https://reviews.apache.org/r/10028/#comment38317>
Move these delete operations into a PathInfo destructor, then these loops can be const& again.
src/slave/gc.cpp
<https://reviews.apache.org/r/10028/#comment38318>
Emphasize that we "must" remove it.
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment38319>
Make this a TemporaryDirectoryTest instead.
- Ben Mahler
On March 19, 2013, 9:02 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 19, 2013, 9:02 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/gc_tests.cpp a9a7a439702ab87d5384c2d1b8afc0b17dab4e62
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18470
-----------------------------------------------------------
Holding off for: https://reviews.apache.org/r/10166/
- Ben Mahler
On March 22, 2013, 11:54 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 22, 2013, 11:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
> On April 8, 2013, 3:22 a.m., Vinod Kone wrote:
> > src/slave/gc.cpp, lines 96-98
> > <https://reviews.apache.org/r/10028/diff/4/?file=276528#file276528line96>
> >
> > Should this be a CHECK?
This is the false case, when the GCProcess doesn't know about the path.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18765
-----------------------------------------------------------
On March 30, 2013, 10:04 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 30, 2013, 10:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp 605957c686efd8e50ee5dfd61de51e192d34ac92
> src/slave/gc.cpp c40f42a78457318a7968a6d50bb06632f289db05
> src/tests/gc_tests.cpp 037fe096179596a6bb9359b4258c2afafee300f9
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18765
-----------------------------------------------------------
src/slave/gc.cpp
<https://reviews.apache.org/r/10028/#comment39175>
Should this be a CHECK?
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment39176>
EXPECT_DISPATCH and EXPECT_MESSAGE (and triggers) have been deprecated, as of last week.
Please rebase of trunk and use FUTURE_DISPATCH.
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment39177>
A Clock::settle() would ensure that GarbageCollectorProcess::schedule will be finished. No need for introducing sleep.
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment39178>
Why not just call gc.unschedule directly here instead of having the temp variables?
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment39179>
ditto
- Vinod Kone
On March 30, 2013, 10:04 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 30, 2013, 10:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp 605957c686efd8e50ee5dfd61de51e192d34ac92
> src/slave/gc.cpp c40f42a78457318a7968a6d50bb06632f289db05
> src/tests/gc_tests.cpp 037fe096179596a6bb9359b4258c2afafee300f9
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
> On April 9, 2013, 4:13 p.m., Benjamin Hindman wrote:
> > src/slave/gc.hpp, line 94
> > <https://reviews.apache.org/r/10028/diff/5/?file=279123#file279123line94>
> >
> > Hmm, better location for the comment but honestly I would still put it next to the actual removal operation. The meat of this TODO is really: do the actual removal asynchronously from the GarbageCollectorProcess.
Agreed, and done.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18856
-----------------------------------------------------------
On April 8, 2013, 11:52 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated April 8, 2013, 11:52 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp 605957c686efd8e50ee5dfd61de51e192d34ac92
> src/slave/gc.cpp c40f42a78457318a7968a6d50bb06632f289db05
> src/tests/gc_tests.cpp 037fe096179596a6bb9359b4258c2afafee300f9
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18856
-----------------------------------------------------------
Ship it!
The multimap cleans this up really nicely!
src/slave/gc.hpp
<https://reviews.apache.org/r/10028/#comment39310>
Hmm, better location for the comment but honestly I would still put it next to the actual removal operation. The meat of this TODO is really: do the actual removal asynchronously from the GarbageCollectorProcess.
- Benjamin Hindman
On April 8, 2013, 11:52 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated April 8, 2013, 11:52 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp 605957c686efd8e50ee5dfd61de51e192d34ac92
> src/slave/gc.cpp c40f42a78457318a7968a6d50bb06632f289db05
> src/tests/gc_tests.cpp 037fe096179596a6bb9359b4258c2afafee300f9
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/
-----------------------------------------------------------
(Updated April 9, 2013, 5:47 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Rebased and benh comment.
Description
-------
See above. This was needed in order to write a fix for MESOS-396.
This addresses bug MESOS-396.
https://issues.apache.org/jira/browse/MESOS-396
Diffs (updated)
-----
src/slave/cgroups_isolator.cpp d31c2a50787058edb99c2b8070446ead128dc79e
src/slave/gc.hpp 605957c686efd8e50ee5dfd61de51e192d34ac92
src/slave/gc.cpp c40f42a78457318a7968a6d50bb06632f289db05
src/tests/gc_tests.cpp 037fe096179596a6bb9359b4258c2afafee300f9
src/tests/utils.hpp 23ea3dd3be5fbc9477f99160e3508d7f09ac359e
Diff: https://reviews.apache.org/r/10028/diff/
Testing
-------
Added missing and new unit tests.
Also ran the tests with 300 iterations.
Thanks,
Ben Mahler
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/
-----------------------------------------------------------
(Updated April 8, 2013, 11:52 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Updated to use the new testing abstractions.
Description
-------
See above. This was needed in order to write a fix for MESOS-396.
This addresses bug MESOS-396.
https://issues.apache.org/jira/browse/MESOS-396
Diffs (updated)
-----
src/slave/gc.hpp 605957c686efd8e50ee5dfd61de51e192d34ac92
src/slave/gc.cpp c40f42a78457318a7968a6d50bb06632f289db05
src/tests/gc_tests.cpp 037fe096179596a6bb9359b4258c2afafee300f9
Diff: https://reviews.apache.org/r/10028/diff/
Testing
-------
Added missing and new unit tests.
Also ran the tests with 300 iterations.
Thanks,
Ben Mahler
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/
-----------------------------------------------------------
(Updated March 30, 2013, 10:04 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
This now has a much cleaner implementation, thanks to Multimap.
This could be simplified further by removing the added hashmap, at the cost of having to do a full Multimap search during schedule() and unschedule().
Ran the tests with 3000 iterations.
Description
-------
See above. This was needed in order to write a fix for MESOS-396.
This addresses bug MESOS-396.
https://issues.apache.org/jira/browse/MESOS-396
Diffs (updated)
-----
src/slave/gc.hpp 605957c686efd8e50ee5dfd61de51e192d34ac92
src/slave/gc.cpp c40f42a78457318a7968a6d50bb06632f289db05
src/tests/gc_tests.cpp 037fe096179596a6bb9359b4258c2afafee300f9
Diff: https://reviews.apache.org/r/10028/diff/
Testing
-------
Added missing and new unit tests.
Also ran the tests with 300 iterations.
Thanks,
Ben Mahler
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Vinod Kone <vi...@gmail.com>.
> On March 25, 2013, 2:17 a.m., Benjamin Hindman wrote:
> > src/slave/gc.hpp, line 64
> > <https://reviews.apache.org/r/10028/diff/3/?file=273944#file273944line64>
> >
> > I don't think this TODO should be here. As far as the API is concerned, you're returning a Future exactly because you might block. If there is a way to optimize 'unschedule' then you should put that TODO there.
+1
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18334
-----------------------------------------------------------
On March 22, 2013, 11:54 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 22, 2013, 11:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18334
-----------------------------------------------------------
src/slave/gc.hpp
<https://reviews.apache.org/r/10028/#comment38541>
I don't think this TODO should be here. As far as the API is concerned, you're returning a Future exactly because you might block. If there is a way to optimize 'unschedule' then you should put that TODO there.
- Benjamin Hindman
On March 22, 2013, 11:54 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 22, 2013, 11:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/
-----------------------------------------------------------
(Updated March 22, 2013, 11:54 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Vinod's review.
Description
-------
See above. This was needed in order to write a fix for MESOS-396.
This addresses bug MESOS-396.
https://issues.apache.org/jira/browse/MESOS-396
Diffs (updated)
-----
src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
Diff: https://reviews.apache.org/r/10028/diff/
Testing
-------
Added missing and new unit tests.
Also ran the tests with 300 iterations.
Thanks,
Ben Mahler
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Vinod Kone <vi...@gmail.com>.
> On March 22, 2013, 10:26 p.m., Vinod Kone wrote:
> > src/slave/gc.cpp, line 149
> > <https://reviews.apache.org/r/10028/diff/2/?file=273646#file273646line149>
> >
> > Why did you have to use the iterator?
> >
> > Considering you are returning after acting on one element, I think you can just use a foreach() loop here.
>
> Ben Mahler wrote:
> As discussed offline, I need to delete from the iterator. Will add a TODO to clean this up by adding multimap.hpp to stout.
as discussed offline, a multi-map should solve this.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18292
-----------------------------------------------------------
On March 22, 2013, 11:54 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 22, 2013, 11:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
> On March 22, 2013, 10:26 p.m., Vinod Kone wrote:
> > src/tests/gc_tests.cpp, line 426
> > <https://reviews.apache.org/r/10028/diff/2/?file=273647#file273647line426>
> >
> > We should probably move these unit tests to the top. Unit tests after integration tests seems weird :)
Agreed, done!
> On March 22, 2013, 10:26 p.m., Vinod Kone wrote:
> > src/tests/gc_tests.cpp, line 474
> > <https://reviews.apache.org/r/10028/diff/2/?file=273647#file273647line474>
> >
> > ditto?
Drop as discussed (accidental comment).
> On March 22, 2013, 10:26 p.m., Vinod Kone wrote:
> > src/slave/gc.cpp, line 149
> > <https://reviews.apache.org/r/10028/diff/2/?file=273646#file273646line149>
> >
> > Why did you have to use the iterator?
> >
> > Considering you are returning after acting on one element, I think you can just use a foreach() loop here.
As discussed offline, I need to delete from the iterator. Will add a TODO to clean this up by adding multimap.hpp to stout.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18292
-----------------------------------------------------------
On March 21, 2013, 11:29 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 21, 2013, 11:29 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/#review18292
-----------------------------------------------------------
src/slave/gc.cpp
<https://reviews.apache.org/r/10028/#comment38479>
Why not CHECK(unschedule(path))?
src/slave/gc.cpp
<https://reviews.apache.org/r/10028/#comment38480>
quotes around path.
src/slave/gc.cpp
<https://reviews.apache.org/r/10028/#comment38481>
Why did you have to use the iterator?
Considering you are returning after acting on one element, I think you can just use a foreach() loop here.
src/slave/gc.cpp
<https://reviews.apache.org/r/10028/#comment38482>
quotes around path.
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment38493>
We should probably move these unit tests to the top. Unit tests after integration tests seems weird :)
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment38484>
s/first GCs/GCs of file1 and file2/
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment38485>
s/final GC/GC of file3/
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment38487>
ditto?
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10028/#comment38492>
s/final file/file4/
- Vinod Kone
On March 21, 2013, 11:29 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10028/
> -----------------------------------------------------------
>
> (Updated March 21, 2013, 11:29 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> See above. This was needed in order to write a fix for MESOS-396.
>
>
> This addresses bug MESOS-396.
> https://issues.apache.org/jira/browse/MESOS-396
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10028/diff/
>
>
> Testing
> -------
>
> Added missing and new unit tests.
>
> Also ran the tests with 300 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added 'unschedule' operation to the GarbageCollector,
changed schedule to discard the future when unscheduled/rescheduled,
added GarbageCollector unit tests.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10028/
-----------------------------------------------------------
(Updated March 21, 2013, 11:29 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
benh review
./bin/mesos-tests.sh --gtest_filter="GarbageCollectorTest.*" --verbose --gtest_repeat=300 --gtest_break_on_failure
Description
-------
See above. This was needed in order to write a fix for MESOS-396.
This addresses bug MESOS-396.
https://issues.apache.org/jira/browse/MESOS-396
Diffs (updated)
-----
src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
Diff: https://reviews.apache.org/r/10028/diff/
Testing
-------
Added missing and new unit tests.
Also ran the tests with 300 iterations.
Thanks,
Ben Mahler