> On Oct. 21, 2014, 6:27 p.m., Ben Mahler wrote: > > src/slave/status_update_manager.cpp, lines 171-175 > > <https://reviews.apache.org/r/26957/diff/1/?file=726661#file726661line171> > > > > Why doesn't pause() clear all the stream timeouts by setting them to > > None()? > > > > Then the timeout() method wouldn't have to explicitly look at 'paused', > > since no timers are expired.
clearing "timeout" seems a bit weird because "timeout" tells whether there is a pending timeout, not whether the SUM is paused. overloading the semantics will be weird. i think the main issue is that forward() returns expired Timeout(). i'll change it so that the callers don't call forward() is SUM is paused. this is more explicit. > On Oct. 21, 2014, 6:27 p.m., Ben Mahler wrote: > > src/slave/status_update_manager.cpp, line 371 > > <https://reviews.apache.org/r/26957/diff/1/?file=726661#file726661line371> > > > > Seems like this should return a None(), so that the stream->timeout is > > None() instead of an expired Timeout. Then forward returns a None() timeout > > when paused. > > > > Or if you don't want to change the signature of forward, then we could > > only call forward when we're not paused, which is more explicit but > > trickier for callers. > > > > Also, why the one-line bracing like this? see above. > On Oct. 21, 2014, 6:27 p.m., Ben Mahler wrote: > > src/slave/status_update_manager.cpp, line 469 > > <https://reviews.apache.org/r/26957/diff/1/?file=726661#file726661line469> > > > > If we set the Timeouts to None() when pausing, we can just remove this > > CHECK and consider None() timers as not expired. see above. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26957/#review57517 ----------------------------------------------------------- On Oct. 20, 2014, 11:51 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26957/ > ----------------------------------------------------------- > > (Updated Oct. 20, 2014, 11:51 p.m.) > > > Review request for mesos and Ben Mahler. > > > Repository: mesos-git > > > Description > ------- > > Renamed flush() to resume() and added pause() to status update manager. > pause() should let status update manager know to not send updates when slave > is disconnected. > > Also updated the constructor of status update manager. > > > Diffs > ----- > > src/local/local.cpp 66de798377269150a2e546daa5ca7c5371bbce79 > src/slave/main.cpp b27cc32ebccb1c97f2f2ae0b904c725bbf541ebf > src/slave/slave.cpp 7b5474ae898f369fc55670cf1dbbee684bc0ae4b > src/slave/status_update_manager.hpp > c371e5542e12cd6bb9401591cd691d99bc0fa8fd > src/slave/status_update_manager.cpp > 5d5cf234ef2dd47fa4b1f67be761dbca31659451 > src/tests/cluster.hpp ee194ad0b0014eb4c415c294cfeaf5f7f3089a7f > src/tests/mesos.hpp e40575c6d330cfa3fbfad2efcf601418c413b992 > src/tests/mesos.cpp 147e23fec94add19f9d5c34e6f7e97874db9b6b2 > > Diff: https://reviews.apache.org/r/26957/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
