----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20572/#review41175 -----------------------------------------------------------
src/local/local.cpp <https://reviews.apache.org/r/20572/#comment74582> Maybe we should say "--work_dir" instead of "work directory"? src/master/flags.hpp <https://reviews.apache.org/r/20572/#comment74586> How about: "It is imperative to set this value to include a majority of masters, i.e. Q > N/2 where N is the number of masters." Not sure whether we want to show C++ code instead of math. src/master/flags.hpp <https://reviews.apache.org/r/20572/#comment74588> "automatically initialize" Maybe a little blurb about the fact that if this is false, one has to manually initialize the log using the tool when running for the first time. src/master/flags.hpp <https://reviews.apache.org/r/20572/#comment74592> Technically we don't shut them down as they may already be shutdown, maybe we should say they will be removed from the registry and we will enforce that they be shut down if they re-communicate with the master? (Trying not to be too verbose but our documentation is lacking so I kind of like the idea of giving operators enough context through our flag descriptions). src/master/main.cpp <https://reviews.apache.org/r/20572/#comment74593> Right now the "strict" validation happens in the Master but we should be really careful when we remove that. Now I'm thinking it might be better to enforce that "in_memory" and "registry_strict" are not used together in main.cpp? How about a TODO on the Master::initialize code so that we know what to do when we remove the "registry_strict" check? src/master/main.cpp <https://reviews.apache.org/r/20572/#comment74595> Your comment in group.cpp says "replicas" but this one uses "log_replicas". src/tests/cluster.hpp <https://reviews.apache.org/r/20572/#comment74598> Ditto here: "replicas" used in group.cpp. src/tests/registrar_zookeeper_tests.cpp <https://reviews.apache.org/r/20572/#comment74601> Awesome, thanks for adding this! src/tests/registrar_zookeeper_tests.cpp <https://reviews.apache.org/r/20572/#comment74600> Could we use createTask() from mesos.hpp? I see that's what we do in slave_recovery_tests.cpp. src/zookeeper/group.cpp <https://reviews.apache.org/r/20572/#comment74596> Might be nice to keep LOG(WARNING) but modify the check: if (sequence.isError() && tokens.back() != "replicas") { Otherwise we might not notice something dumping nodes in our path? - Ben Mahler On April 23, 2014, 6:06 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20572/ > ----------------------------------------------------------- > > (Updated April 23, 2014, 6:06 p.m.) > > > Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-1226 > https://issues.apache.org/jira/browse/MESOS-1226 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d > src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f > src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 > src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 > src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 > src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 > src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f > src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d > src/tests/balloon_framework_test.sh > f83240758b03871b8b53f45d0947c6171c9c3a93 > src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a > src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de > src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 > src/tests/registrar_zookeeper_tests.cpp PRE-CREATION > src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d > src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e > > Diff: https://reviews.apache.org/r/20572/diff/ > > > Testing > ------- > > Added a new unit test that tests mesos cluster with registrar and zookeeper. > > Also, updated external tests to use log storage but without zookeeper. > > make check > > > Thanks, > > Vinod Kone > >
