> 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?
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. - Ben ----------------------------------------------------------- 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 > >
