----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36497/#review91934 -----------------------------------------------------------
src/sched/sched.cpp (lines 453 - 454) <https://reviews.apache.org/r/36497/#comment145667> Do you want to add a CHECK to make sure that 'master' is also None? I propose that we get rid of 'master' altogether. See my comment below. src/sched/sched.cpp (line 463) <https://reviews.apache.org/r/36497/#comment145669> except for the 3rd case. can you add that to the comment. you can copy paste what you wrote in the description of this review. src/sched/sched.cpp (line 1393) <https://reviews.apache.org/r/36497/#comment145665> Why store both 'master' and 'masterInfo'? I think you can get rid of 'master'. Gets us away from having to make sure they are in sync. src/tests/scheduler_event_call_tests.cpp (line 59) <https://reviews.apache.org/r/36497/#comment145670> Can you expand on the comment here? This test is a bit complicated and could use some comments on what you are doing and testing. src/tests/scheduler_event_call_tests.cpp (line 73) <https://reviews.apache.org/r/36497/#comment145674> Needs a comment on why you are dropping the message. src/tests/scheduler_event_call_tests.cpp (line 76) <https://reviews.apache.org/r/36497/#comment145681> why pausing the clock? comment. src/tests/scheduler_event_call_tests.cpp (lines 83 - 84) <https://reviews.apache.org/r/36497/#comment145671> Why are you doing this test? src/tests/scheduler_event_call_tests.cpp (lines 89 - 91) <https://reviews.apache.org/r/36497/#comment145673> pull this below the expectation. src/tests/scheduler_event_call_tests.cpp (line 97) <https://reviews.apache.org/r/36497/#comment145672> comment. src/tests/scheduler_event_call_tests.cpp (line 101) <https://reviews.apache.org/r/36497/#comment145675> s/call/message/ src/tests/scheduler_event_call_tests.cpp (line 115) <https://reviews.apache.org/r/36497/#comment145676> comment. src/tests/scheduler_event_call_tests.cpp (lines 119 - 121) <https://reviews.apache.org/r/36497/#comment145677> hmm. can you split this into its own test. this test is huge! src/tests/scheduler_event_call_tests.cpp (line 139) <https://reviews.apache.org/r/36497/#comment145679> comment. src/tests/scheduler_event_call_tests.cpp (line 145) <https://reviews.apache.org/r/36497/#comment145678> ditto. split this into its own test. src/tests/scheduler_event_call_tests.cpp (line 158) <https://reviews.apache.org/r/36497/#comment145680> comment. - Vinod Kone On July 15, 2015, 1:47 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36497/ > ----------------------------------------------------------- > > (Updated July 15, 2015, 1:47 a.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-2910 > https://issues.apache.org/jira/browse/MESOS-2910 > > > Repository: mesos > > > Description > ------- > > This one is non-trivial, note that we follow MESOS-786 with the exception of > the 3rd case, since it is not possible and schedulers could not have possibly > relied on getting registered instead of re-registered in that case. > > We now need to store the MasterInfo coming from the detector, as it is not > provided in Event. > > > Diffs > ----- > > src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f > src/tests/scheduler_event_call_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/36497/diff/ > > > Testing > ------- > > Added a test. > > > Thanks, > > Ben Mahler > >
