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