> On Feb. 18, 2014, 6:46 p.m., Vinod Kone wrote: > > src/master/registry.proto, lines 27-28 > > <https://reviews.apache.org/r/18158/diff/1/?file=486550#file486550line27> > > > > Why not > > > > repeated<Slave> slaves = 2; ? > > > > I think that would make some of the code in registry.cpp more readable. > > > > For example, > > > > s/registry.slaves.slaves().size()/registry.slaves().size()/ > > > >
This is a good point, the reason I didn't do this was because we were originally storing a "slaves" key in the Registry state to atomically modify slaves. Now we use a single "registry" key for all Registry information and I agree the 'Slaves' wrapper ends up being a bit clunky in the C++ code. There are a few reasons I kept the 'Slaves' wrapper: 1. In the C++, if one wants to pass around the slaves of the Registry, one would have to use a 'RepeatedPtrField<Slave>' rather than just passing around 'Slaves', this could end up being pretty clunky as well. 2. Less realistically, as the Registry grows, we may want to abandon the single "registry" key model and store separate keys for "slaves", "master", etc. Keeping a 'Slaves' wrapper protobuf makes this much easier to do, since the protobuf message for slaves would not change. I've left your issue open, let me know what you think! - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18158/#review34731 ----------------------------------------------------------- On Feb. 15, 2014, 12:57 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18158/ > ----------------------------------------------------------- > > (Updated Feb. 15, 2014, 12:57 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Bugs: MESOS-764 > https://issues.apache.org/jira/browse/MESOS-764 > > > Repository: mesos-git > > > Description > ------- > > The registrar now uses a single Variable for the Registry, as outlined within: > https://cwiki.apache.org/confluence/display/MESOS/Registrar+Design+Document > > As a heads up, there are a number of changes / fixes that will follow this as > well, I am attempting to break these changes apart. > > > Diffs > ----- > > src/master/registrar.cpp 915885a160f790399e8185c28c6e6555af1ee76e > src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3 > > Diff: https://reviews.apache.org/r/18158/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
