> On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/cli/resolve.cpp, line 134 > > <https://reviews.apache.org/r/13087/diff/9/?file=373358#file373358line134> > > > > Is this true?
The destructor fails the future but it is not the case here. Here a Result::error() is returned for failure cases. > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/local/local.cpp, lines 105-106 > > <https://reviews.apache.org/r/13087/diff/9/?file=373361#file373361line105> > > > > Can we give them both the same name (Single or Standalone) for symmetry? > > > > I initially thought there are some subtle differences in their behavior thus named differently but ok... for the sake of symmetry > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/contender.hpp, line 62 > > <https://reviews.apache.org/r/13087/diff/9/?file=373363#file373363line62> > > > > s/zk/master/ ? "master" is a bit strange here because it can never be anything other than a ZooKeeper string or empty string. (It doesn't take a host:port as the detector does). And the variable name "zk" corresponds to the command line flag "zk" in master/main.cpp which is the only place this method is used. > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/contender.cpp, line 56 > > <https://reviews.apache.org/r/13087/diff/9/?file=373364#file373364line56> > > > > virtual? It's not inheriting the virtual initialize() from Process. I thought about using "init()" to avoid confusion but decided not to. > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/detector.cpp, line 177 > > <https://reviews.apache.org/r/13087/diff/9/?file=373366#file373366line177> > > > > s/INFO/WARNING/ ? This is when appoint() is called before detect() which happens pretty often so maybe not a warning? > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/detector.cpp, line 274 > > <https://reviews.apache.org/r/13087/diff/9/?file=373366#file373366line274> > > > > s/previous/expected/ Discussed with Vinod are will use 'previous' in all instances. > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/master.hpp, line 201 > > <https://reviews.apache.org/r/13087/diff/9/?file=373369#file373369line201> > > > > s/lostCandidacy/lost/ ? Even though "lost" is currently an available name in Master but the Master can "lose" a lot of types of things right? And the parameter type "Future<Nothing>" is not very descriptive here either... > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/master.cpp, line 705 > > <https://reviews.apache.org/r/13087/diff/9/?file=373370#file373370line705> > > > > why "somehow"? This should not happen, thus LOG(FATAL). Changed to use "CHECK". > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/master.cpp, line 727 > > <https://reviews.apache.org/r/13087/diff/9/?file=373370#file373370line727> > > > > are you missing a return here? > > > > Also, is this expected? No. A failed future happens when the Group disconnects, which qualifies as an error (but expected) case and we add a log line here, but whether to contend again is decided below so we don't directly return here. > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/detector.hpp, line 63 > > <https://reviews.apache.org/r/13087/diff/9/?file=373365#file373365line63> > > > > When does the future fail/get discarded? Added a line for this. > On Oct. 30, 2013, 1:51 a.m., Vinod Kone wrote: > > src/master/detector.cpp, line 189 > > <https://reviews.apache.org/r/13087/diff/9/?file=373366#file373366line189> > > > > s/pid/expected/ ? Did s/pid/previous/ - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13087/#review27716 ----------------------------------------------------------- On Oct. 29, 2013, 5:02 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13087/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2013, 5:02 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-496 > https://issues.apache.org/jira/browse/MESOS-496 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/scheduler.hpp fa1ffe8b24a054556593d31eeae3fd1ac1761fb5 > src/Makefile.am 0b32d74e43d21eb2f77b0abd8864ba5f8cf46214 > src/cli/resolve.cpp b05f5103ca898f74acb7dfe8f423ffc7e1872e09 > src/detector/detector.hpp 3aaebfe89f0b4bb5c64e7f4c8974d8b13bc6f6b1 > src/detector/detector.cpp 8d9f118757192ca38bcdb26663ed86effbcf3c9e > src/local/local.cpp e4b5ec5b3dfae6dd89746353d3caaa8d7e117840 > src/local/main.cpp 5995c538077dea301f231668b4b5f905e73e7b3b > src/master/contender.hpp PRE-CREATION > src/master/contender.cpp PRE-CREATION > src/master/detector.hpp PRE-CREATION > src/master/detector.cpp PRE-CREATION > src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 > src/master/main.cpp 19fcb9f09d8fc03a0719aada7d216b575eb3069b > src/master/master.hpp 1eba03fc4fc2ae8b75603a241c431c7a4fe2249e > src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 > src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b > src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c > src/slave/http.cpp 62fbb37a1924062543bf9db4229704bdef91601d > src/slave/main.cpp 750a12766bde64059bfd4635ea077cbd43cb4301 > src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb > src/slave/slave.cpp 06ef7860bf0d9531b50a504af0a46f6d2b900855 > src/tests/allocator_tests.cpp b0beb72273e9d44a407d0e83e6c18a541b212089 > src/tests/authentication_tests.cpp 48a9323d03416ad9ee25fc19838d89678ff613bc > src/tests/cluster.hpp 5621139df44e3e2ff330bdbe77fe3dda0165e143 > src/tests/fault_tolerance_tests.cpp > 3989e14eb926d7f9b5c7731aa0713a56fc3fb6e9 > src/tests/gc_tests.cpp 5459b78126d3607114ea171d3f76883f8355751c > src/tests/isolator_tests.cpp ab5b00aa7f12dbd299debc1a405ac50c59cec85c > src/tests/master_contender_detector_tests.cpp PRE-CREATION > src/tests/master_detector_tests.cpp > 06c586d29996cd599ae58dfd49cf1324aac0a6d6 > src/tests/master_tests.cpp bf790d24ff6b2fb199a04caebef523797b075e63 > src/tests/mesos.hpp ad39c095f9699fbe7958d52698fd2b0319e84eb9 > src/tests/mesos.cpp 10b3e5381af49cf07e0d0a6f457d646e6e6718ca > src/tests/slave_recovery_tests.cpp 2812f110925505258a291b72f0d6525ee56987c7 > > Diff: https://reviews.apache.org/r/13087/diff/ > > > Testing > ------- > > make check 100 times > > > Thanks, > > Jiang Yan Xu > >
