> On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 160 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line160> > > > > Can you place a CHECK_EQ(state, INITIAL) before setting the state to > > ELECTING?
Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, lines 178-190 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line178> > > > > Can you move demote() below all of the election related functions? Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 62 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line62> > > > > s/committed/committed (learned)/ might be helpful to those familiar > > with paxos but unfamiliar with our terminology, ditto elsewhere Since I will move these comments else where in the next patch, I will do that in the next patch. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, lines 67-69 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line67> > > > > 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". ditto > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 188 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line188> > > > > CHECK_EQ(state, ELECTED) before this? Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 225 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line225> > > > > CHECK_LE? Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 281 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line281> > > > > CHECK_EQ? Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 293 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line293> > > > > CHECK_EQ? Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 298 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line298> > > > > s/Aborted/Discarded/ on these continuations? I would prefer the old name because it's more clear that we're gonna cancel the election. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 302 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line302> > > > > CHECK_EQ? Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, lines 307-309 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line307> > > > > Why not use these same blocks to delineate the election and demotion > > parts of the implementation as well? Done > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 381 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line381> > > > > CHECK_EQ? Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, line 412 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line412> > > > > CHECK_LE? Done. > On Dec. 2, 2013, 1:28 a.m., Ben Mahler wrote: > > src/log/coordinator.cpp, lines 453-471 > > <https://reviews.apache.org/r/14902/diff/9/?file=390614#file390614line453> > > > > CHECK_EQ on these? Done. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14902/#review29573 ----------------------------------------------------------- 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 > >
