> On Sept. 21, 2014, 9:20 a.m., Adam B wrote: > > src/master/master.hpp, lines 947-950 > > <https://reviews.apache.org/r/25866/diff/2/?file=699118#file699118line947> > > > > Comment here should be for the Slave. > > 'active' is set to false if resources from this slave should not be > > offered. This happens when the slave is disconnected or the master receives > > a DeactivateSlaveMessage. > > Do we have a DeactivateSlaveMessage yet? Or does that come with the > > HTTP endpoint?
oops. good catch. fixed. we don't have a DeactivateSlaveMessage yet. not sure when it's going to be introduced. > On Sept. 21, 2014, 9:20 a.m., Adam B wrote: > > src/master/master.cpp, lines 3061-3062 > > <https://reviews.apache.org/r/25866/diff/2/?file=699119#file699119line3061> > > > > Why are these CHECKs? How should the master respond if it does receive > > such a message from a deactivated slave? Should we perhaps be sending a > > Shutdown[Slave]Message, or some sort of DeactivateSlaveMessage? This is a CHECK because it shouldn't happen with the current design (slave is deactivated only when disconnected). When these semantics change (e.g., introduction of DeactivateSlaveMessage) we'll have to change the CHECK. > On Sept. 21, 2014, 9:20 a.m., Adam B wrote: > > src/master/master.cpp, line 1552 > > <https://reviews.apache.org/r/25866/diff/2/?file=699119#file699119line1552> > > > > Can you explain why this TODO is no longer needed? > > allocator->frameworkActivated calls allocator->allocate, which will sort > > roles/frameworks and make initial offers based on a stale notion of the > > newly reactivated framework's outstanding offers. If the resources were > > recovered first, the allocator would make fairer offers when the framework > > is first reactivated. It was a bad TODO (on my part) because it doesn't tell me why I didn't fix it in the first place instead of adding a TODO. Maybe I added the TODO as part of some other cleanup and didn't want to change too much in that review. Anyway, now I addressed the TODO now. > On Sept. 21, 2014, 9:20 a.m., Adam B wrote: > > src/master/master.cpp, line 3988 > > <https://reviews.apache.org/r/25866/diff/2/?file=699119#file699119line3988> > > > > Ditto. Isn't this TODO still relevant? fixed. see above. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25866/#review54094 ----------------------------------------------------------- On Sept. 20, 2014, 6:46 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25866/ > ----------------------------------------------------------- > > (Updated Sept. 20, 2014, 6:46 p.m.) > > > Review request for mesos, Adam B and Ben Mahler. > > > Bugs: MESOS-1081 and MESOS-1811 > https://issues.apache.org/jira/browse/MESOS-1081 > https://issues.apache.org/jira/browse/MESOS-1811 > > > Repository: mesos-git > > > Description > ------- > > Made consistent what connected and active frameworks/slaves means. > > Fixed MESOS-1811 along the way. > > > Diffs > ----- > > src/master/http.cpp 3f5a01dfddca9cea73563100d88e0c03f600d6b1 > src/master/master.hpp f5d74aef185fad861139186be1cab089f8005a94 > src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 > src/tests/fault_tolerance_tests.cpp > 154386044d0247b39d84719d7ff14250682a0695 > > Diff: https://reviews.apache.org/r/25866/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
