----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14902/#review29573 -----------------------------------------------------------
Very nice, I will let benh give a ship it for this, looks good to me! src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56943> s/committed/committed (learned)/ might be helpful to those familiar with paxos but unfamiliar with our terminology, ditto elsewhere src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56948> Perhaps a note here (and/or in the header) that one can only demote() once the co-ordinator has been fully elected? That is: elect(); demote(); // Possibly fails. elect().get(); demote(); // Expected usage. Ditto for adding a note with respect to not being able to demote while a write is "in progress". src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56940> Can you place a CHECK_EQ(state, INITIAL) before setting the state to ELECTING? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56945> Can you move demote() below all of the election related functions? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56941> CHECK_EQ(state, ELECTED) before this? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56946> CHECK_LE? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56947> CHECK_EQ? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56950> CHECK_EQ? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56952> s/Aborted/Discarded/ on these continuations? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56951> CHECK_EQ? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56959> Why not use these same blocks to delineate the election and demotion parts of the implementation as well? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56954> CHECK_EQ? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56955> CHECK_LE? src/log/coordinator.cpp <https://reviews.apache.org/r/14902/#comment56956> CHECK_EQ on these? - Ben Mahler On Nov. 25, 2013, 5:53 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14902/ > ----------------------------------------------------------- > > (Updated Nov. 25, 2013, 5:53 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-736 > https://issues.apache.org/jira/browse/MESOS-736 > > > Repository: mesos-git > > > Description > ------- > > Libprocessify the coordinator. > > > Diffs > ----- > > src/log/coordinator.hpp 3f6fb7c > src/log/coordinator.cpp 6e6466f > > Diff: https://reviews.apache.org/r/14902/diff/ > > > Testing > ------- > > bin/mesos-tests.sh --gtest_filter=*CoordinatorTest*:*LogTest*:*ReplicaTest* > --gtest_repeat=100 > > > Thanks, > > Jie Yu > >
