----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50701/#review146394 -----------------------------------------------------------
src/master/master.hpp (line 1907) <https://reviews.apache.org/r/50701/#comment212800> do you want to get rid of this parameter (in a different review and for all operations) because this is no longer configurable? src/master/master.hpp (line 1949) <https://reviews.apache.org/r/50701/#comment212803> i think you are using "registered" and "admitted" interchangeably ?, which is a bit confusing. the master has different variables for "recovered", "registering", "reregistering", "registered", "removing" and "removed". in this particular case, does the master mark admitted but not currently registered agents as unreachable? if yes, s/registered/admitted/ ? src/master/master.hpp (line 1951) <https://reviews.apache.org/r/50701/#comment212845> since reachable agents can be marked as reachable again, why not allow unreachable agents to be marked as unreachable again? src/master/master.hpp (line 1976) <https://reviews.apache.org/r/50701/#comment212804> looks like only 'id' is ever used. s/SlaveInfo info/SlaveID id/ if you want to keep SlaveInfo for consistency with other operations, that's fine as well. src/master/master.hpp (line 1980) <https://reviews.apache.org/r/50701/#comment212828> per above comments, "admitted" doesn't mean "registered" right, esp after a failover. src/master/master.hpp (line 1982) <https://reviews.apache.org/r/50701/#comment212829> s/registered/admitted/ src/master/master.hpp (line 1983) <https://reviews.apache.org/r/50701/#comment212830> s/also still be registered/have already been admitted/ ? src/master/master.hpp (line 1984) <https://reviews.apache.org/r/50701/#comment212805> s/if it its/if its/ s/registered/admitted/ src/master/master.hpp (line 1986) <https://reviews.apache.org/r/50701/#comment212842> I think calling this operation "Readmit" is more appropriate because you are adding it back to admitted list? src/master/master.hpp (line 2000) <https://reviews.apache.org/r/50701/#comment212831> s/:/;/ ? src/master/master.hpp (line 2027) <https://reviews.apache.org/r/50701/#comment212834> s/reachable/admitted/ src/tests/registrar_tests.cpp (line 311) <https://reviews.apache.org/r/50701/#comment212843> s/reregister/readmitted/ - Vinod Kone On Aug. 12, 2016, 12:22 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50701/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2016, 12:22 a.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-4049 > https://issues.apache.org/jira/browse/MESOS-4049 > > > Repository: mesos > > > Description > ------- > > Added registrar operations for marking agents (un-)reachable. > > > Diffs > ----- > > src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad > src/tests/registrar_tests.cpp 9a71d8fd0c8d8e662a5e364015d144396a0b1a4c > > Diff: https://reviews.apache.org/r/50701/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Neil Conway > >
