> 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()/ > > > > > > Ben Mahler wrote: > 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! > > Vinod Kone wrote: > I agree with both the points but (1) is not an issue today (AFACIT, > Slaves* is never being passed around) and (2) doesn't seem to be realistic > future direction considering atomicity requirements? Should we keep it simple > for now and revisit if the need arises? I understand changing protobufs is > hard, so want to make sure our arguments for this decision are reasonable. > For example, do you imagine something that ties all the slaves that is going > to go into the Slaves protobuf? > > Ben Mahler wrote: > Currently, per the design doc, we're providing very strong atomicity > requirements to simplify the design of the Registrar. We do not want to have > to think through the implications of storing the Registry across multiple > keys. More so than atomicity, we're using this design to force the failure of > a rogue Master that tries to perform a write. However, as the Registry grows, > this single key design may become cost prohibitive. > > The first optimizations we will do will be to write "diffs" of the > Registry blob to the replicated log, rather than writing the full state. > However, if we begin to store things like global reservations, framework > information, we may want to split the Registry across keys. > > I would be ok avoiding wrapper messages in the Registry, since I think > the need to split keys is sufficiently far in the future. But, I also would > really like to avoid digging ourselves into a hole and making migration > difficult. > > Vinod Kone wrote: > I would imagine if you were to split the registry for space > considerations you would want to split "Slaves" too? If yes, would having a > wrapper make it easy or hard? > > Ben Mahler wrote: > I think it's beneficial to think of the Registry as key/value data > ('slaves', 'frameworks', 'reservations', etc..). This is the model provided > by the State abstraction and as such is the model we should be designing > against. > > This is why I'd like to have an explicit 'slaves' key to capture all > slaves in the Registry as a single 'value'. I think we should avoid splitting > single keys as much as possible. Rather than splitting 'slaves' across > multiple keys to handle larger clusters, we should first look at performance > optimizations (like writing "diffs" in the log, reducing protobuf copying in > the Registrar, etc). Make sense?
sgtm. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18158/#review34731 ----------------------------------------------------------- On Feb. 20, 2014, 4:16 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18158/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2014, 4:16 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 > src/tests/state_tests.cpp 03c538861a88d3a07e2468dce5553eeb3acc9243 > > Diff: https://reviews.apache.org/r/18158/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
