> On Aug. 1, 2013, 6:48 a.m., Benjamin Hindman wrote: > > src/slave/slave.cpp, line 865 > > <https://reviews.apache.org/r/13187/diff/1/?file=332291#file332291line865> > > > > Remove framework framework. ;) Why the change?
The reason I initially changed it was because I was writing a test where I wanted to catch a dispatch on it. Since 'remove' was an overloaded function FUTURE_DISPATCH(_, &Slave::remove) complained. I resolved it by calling FUTURE_DISPATCH(_, (void)(Slave::*)(Framework*, Executor*) &Slave::remove), which satisfied the compiler. But I still wasn't sure if the canonicalization done by FUTURE_DISPATCH would correctly catch it. So I ended up renaming the overloaded remove() for ease of catching the dispatch. I ended up reverting this test because it was getting too complicated. Mainly because I realized that remove(Framework*, Executor*) was never dispatched but rather just directly called as a function. I would've reverted the rename too but looking at other methods in this file, they all seemed to follow the new convention. shutdownFramework() shutdownExecutor() recoverFramework() recoverExecutor() recoverTask() So I decided to keep the rename for consistency. I am ok with renaming all of these into shorter names for consistency. Not sure if we want to do it in this review or not. > On Aug. 1, 2013, 6:48 a.m., Benjamin Hindman wrote: > > src/slave/status_update_manager.hpp, line 110 > > <https://reviews.apache.org/r/13187/diff/1/?file=332292#file332292line110> > > > > Any reason not to flatten these: Future<Try<bool>> => Future<bool>? good call. i will flatten all the status update manager functions. will do that in a subsequent review though. sg? - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13187/#review24419 ----------------------------------------------------------- On Aug. 1, 2013, 5:30 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13187/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2013, 5:30 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-606 > https://issues.apache.org/jira/browse/MESOS-606 > > > Repository: mesos-git > > > Description > ------- > > Changed the semantics of StatusUpdateStream::acknowedgement()'s return value. > > Instead of informing about duplicate, it now informs whether the status > update stream is terminated or not. This is definitely more powerful/useful > information for slave to have. Duplicates are now considered Errors. > > > Diffs > ----- > > src/slave/slave.hpp 97f2083db0dec2c1c9415f65d9bf8a834653bd18 > src/slave/slave.cpp 7f6e6b456890db438092f19a22e4dd816bb33d04 > src/slave/status_update_manager.hpp > 795e74c2b88a071eb7ba720118e06077b6e41238 > src/slave/status_update_manager.cpp > 9e9e4e2a47a609d65ed69a57de595852144a86c8 > src/tests/slave_recovery_tests.cpp 1871e3ba41e65dcbd4b95779dda068f6a1a2ecb3 > src/tests/status_update_manager_tests.cpp > 42395324dfe49659bee2229c6573ffef0874d923 > > Diff: https://reviews.apache.org/r/13187/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
