> On March 12, 2014, 4:17 p.m., Dominic Hamon wrote: > > src/master/registrar.hpp, line 59 > > <https://reviews.apache.org/r/19085/diff/1/?file=516860#file516860line59> > > > > an enum with STRICT/NONSTRICT would be clearer from an API point of > > view.
The callers will be directly using a flag: registrar.admit(slave, flags.registry_strict_operations) An enum would be more clumsy in the caller. > On March 12, 2014, 4:17 p.m., Dominic Hamon wrote: > > src/master/registrar.cpp, line 158 > > <https://reviews.apache.org/r/19085/diff/1/?file=516861#file516861line158> > > > > no need for else as you return in the if. I want to make the difference between strict/non-strict operations very clear for those reading this code, so I would prefer an if / else to make the two cases clear. > On March 12, 2014, 4:17 p.m., Dominic Hamon wrote: > > src/tests/registrar_tests.cpp, line 176 > > <https://reviews.apache.org/r/19085/diff/1/?file=516862#file516862line176> > > > > AWAIT_EQ(!strict, registrar.readmit(info2, strict)); I originally had this. While it is technically correct, it requires some thought to understand. Again in this case I want to make strict / non-strict cases very explicit. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19085/#review36925 ----------------------------------------------------------- On March 12, 2014, 12:45 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19085/ > ----------------------------------------------------------- > > (Updated March 12, 2014, 12:45 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 > ------- > > This adds the notion of "non-strict" operations in the Registrar. Operations > that are not "strict" are always permitted. > > This allows the state to be bootstrapped from the current cluster state and > is essential for those upgrading a running cluster from a pre-Registrar > version of Mesos. > > I have included a TODO that discusses a simplified design of the Registrar > per a chat with benh. I'd like to implement this simplified design in a > subsequent change so I'd like to hear if there are any potential issues. The > new design would mean pushing Operations out to a header and allowing callers > to use Operations directly. > > > Diffs > ----- > > src/master/registrar.hpp 987a63b33bd26add3253f25f1471dd4a73471830 > src/master/registrar.cpp d30172cee8525a80661135e6554bc0d78c7e314d > src/tests/registrar_tests.cpp 95a6b53e479ea9131aca9b39f096e6dc9e6848f8 > > Diff: https://reviews.apache.org/r/19085/diff/ > > > Testing > ------- > > make check > > I've parameterized the existing tests for strict and non-strict operation. > I've also added a test to ensure that the state can be bootstrapped. > > > Thanks, > > Ben Mahler > >
