You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2014/03/31 09:09:32 UTC
Review Request 19834: Cleaned up master detector usage in tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19834/
-----------------------------------------------------------
Review request for mesos, Ben Mahler, Charlie Carson, Vinod Kone, and Jiang Yan Xu.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/tests/cluster.hpp 11684d99c6a4e623dd5ff9977d210de59f33f5cd
src/tests/fault_tolerance_tests.cpp 99311c3e758c52cae02c8bbbd65ad9a8e2016409
src/tests/master_contender_detector_tests.cpp 8da7420e18c7a960b566fae13a5975857eb777ee
src/tests/master_tests.cpp 599f4a0bbe32e36d29f9f08fd8cb6264049c99d5
src/tests/mesos.hpp f77fbfeeaf85f6831c97e45d2163d615c6993df1
src/tests/mesos.cpp ae3aeeeedb83d96f48648621beda6b91179deb0b
Diff: https://reviews.apache.org/r/19834/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 19834: Cleaned up master detector usage in tests.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19834/#review39105
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19834]
All tests passed.
- Mesos ReviewBot
On March 31, 2014, 7:09 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19834/
> -----------------------------------------------------------
>
> (Updated March 31, 2014, 7:09 a.m.)
>
>
> Review request for mesos, Ben Mahler, Charlie Carson, Vinod Kone, and Jiang Yan Xu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 11684d99c6a4e623dd5ff9977d210de59f33f5cd
> src/tests/fault_tolerance_tests.cpp 99311c3e758c52cae02c8bbbd65ad9a8e2016409
> src/tests/master_contender_detector_tests.cpp 8da7420e18c7a960b566fae13a5975857eb777ee
> src/tests/master_tests.cpp 599f4a0bbe32e36d29f9f08fd8cb6264049c99d5
> src/tests/mesos.hpp f77fbfeeaf85f6831c97e45d2163d615c6993df1
> src/tests/mesos.cpp ae3aeeeedb83d96f48648621beda6b91179deb0b
>
> Diff: https://reviews.apache.org/r/19834/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 19834: Cleaned up master detector usage in tests.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On March 31, 2014, 7:09 p.m., Jiang Yan Xu wrote:
> > src/tests/cluster.hpp, lines 195-197
> > <https://reviews.apache.org/r/19834/diff/1/?file=542186#file542186line195>
> >
> > LGTM.
> >
> > Just wondering if for the sake of symmetry we should now treat 'detector' the same way as 'containerizer':
> >
> > // Only register the detector here if it is created within the Cluster.
> > MasterDetector* detector;
> >
> > And delete it explicitly.
> >
> > Also the Owned<MasterDetector> semantics is not followed strictly here: we pass a raw pointer to the Slave (Line 450).
We'd have to refactor Masters::detector to not return an Owned, which I'd prefer to leave for a follow up patch.
I agree that the Owned semantics are not followed strictly (and the above mentioned refactor would solve this) but the improvement here is really to reduce the number of places the Owned semantics are not being followed to just within cluster.hpp!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19834/#review39072
-----------------------------------------------------------
On March 31, 2014, 7:09 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19834/
> -----------------------------------------------------------
>
> (Updated March 31, 2014, 7:09 a.m.)
>
>
> Review request for mesos, Ben Mahler, Charlie Carson, Vinod Kone, and Jiang Yan Xu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 11684d99c6a4e623dd5ff9977d210de59f33f5cd
> src/tests/fault_tolerance_tests.cpp 99311c3e758c52cae02c8bbbd65ad9a8e2016409
> src/tests/master_contender_detector_tests.cpp 8da7420e18c7a960b566fae13a5975857eb777ee
> src/tests/master_tests.cpp 599f4a0bbe32e36d29f9f08fd8cb6264049c99d5
> src/tests/mesos.hpp f77fbfeeaf85f6831c97e45d2163d615c6993df1
> src/tests/mesos.cpp ae3aeeeedb83d96f48648621beda6b91179deb0b
>
> Diff: https://reviews.apache.org/r/19834/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 19834: Cleaned up master detector usage in tests.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19834/#review39072
-----------------------------------------------------------
Ship it!
src/tests/cluster.hpp
<https://reviews.apache.org/r/19834/#comment71474>
LGTM.
Just wondering if for the sake of symmetry we should now treat 'detector' the same way as 'containerizer':
// Only register the detector here if it is created within the Cluster.
MasterDetector* detector;
And delete it explicitly.
Also the Owned<MasterDetector> semantics is not followed strictly here: we pass a raw pointer to the Slave (Line 450).
- Jiang Yan Xu
On March 31, 2014, 12:09 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19834/
> -----------------------------------------------------------
>
> (Updated March 31, 2014, 12:09 a.m.)
>
>
> Review request for mesos, Ben Mahler, Charlie Carson, Vinod Kone, and Jiang Yan Xu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 11684d99c6a4e623dd5ff9977d210de59f33f5cd
> src/tests/fault_tolerance_tests.cpp 99311c3e758c52cae02c8bbbd65ad9a8e2016409
> src/tests/master_contender_detector_tests.cpp 8da7420e18c7a960b566fae13a5975857eb777ee
> src/tests/master_tests.cpp 599f4a0bbe32e36d29f9f08fd8cb6264049c99d5
> src/tests/mesos.hpp f77fbfeeaf85f6831c97e45d2163d615c6993df1
> src/tests/mesos.cpp ae3aeeeedb83d96f48648621beda6b91179deb0b
>
> Diff: https://reviews.apache.org/r/19834/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 19834: Cleaned up master detector usage in tests.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19834/#review39117
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On March 31, 2014, 7:09 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19834/
> -----------------------------------------------------------
>
> (Updated March 31, 2014, 7:09 a.m.)
>
>
> Review request for mesos, Ben Mahler, Charlie Carson, Vinod Kone, and Jiang Yan Xu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 11684d99c6a4e623dd5ff9977d210de59f33f5cd
> src/tests/fault_tolerance_tests.cpp 99311c3e758c52cae02c8bbbd65ad9a8e2016409
> src/tests/master_contender_detector_tests.cpp 8da7420e18c7a960b566fae13a5975857eb777ee
> src/tests/master_tests.cpp 599f4a0bbe32e36d29f9f08fd8cb6264049c99d5
> src/tests/mesos.hpp f77fbfeeaf85f6831c97e45d2163d615c6993df1
> src/tests/mesos.cpp ae3aeeeedb83d96f48648621beda6b91179deb0b
>
> Diff: https://reviews.apache.org/r/19834/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>