> On Feb. 21, 2014, 8:06 p.m., Dominic Hamon wrote: > > src/tests/cluster.hpp, line 320 > > <https://reviews.apache.org/r/18370/diff/1/?file=500334#file500334line320> > > > > it seems odd that we're not setting allocatorProcess to the passed in > > allocatorProcess. If this is to extend the lifetime, maybe the allocator > > should own the allocator process?
There is a private struct Cluster::Masters::Master which just tracks all of the default components which are allocated by Cluster::Masters::start in order to be passed into the real mesos::internal::master::Master. The default components are owned by the Masters object and are released when Masters::stop is called. When start get a specific component (i.e. a Mock Allocator) passed in instead, it shouldn't delete it b/c the lifetime should be controlled by the caller who allocated it. I think the best fix would be to rename Cluster::Masters::Master to be Cluster::Masters::OwnedComponents (a better name suggestion is welcome) or something like that. Does that make sense? - Charlie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18370/#review35187 ----------------------------------------------------------- On Feb. 21, 2014, 8 p.m., Charlie Carson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18370/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2014, 8 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-1024 > https://issues.apache.org/jira/browse/MESOS-1024 > > > Repository: mesos-git > > > Description > ------- > > Refactor Cluster::Master::start methods > > There are currently two overloads of the Cluster::Masters::start > function. One takes a argument of AllocatorProcess* and the other > does not. The AllocatorProcess* overload serves two purposes: > 1) it allows an alternative implementation of AllocatorProcess to be > passed (i.e. a mock) > 2) it changes the destruction timing so that the passed in argument > can outlive the master. > > Beyond that, the two functions are identical. This changes the > parameter to be Option<AllocatorProcess*> and allows all the logic > to be in one method. The old function exists for back-compat but > now simply forward by passing in None() to the other function. > > This is for two purposes: > 1) reduce code duplication > 2) position the code so that we can also optionally pass in a mock > repaier. > > Review: https://reviews.apache.org/r/18370 > > > Diffs > ----- > > src/tests/cluster.hpp d1bf680ed3f6a0fb16af85a21409d653d44522ca > > Diff: https://reviews.apache.org/r/18370/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Charlie Carson > >
