----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55576/#review166216 -----------------------------------------------------------
Looks good, thanks! Can you split the `timedout.clear()` change from the test changes? These seem orthogonal to me, and for posterity it would be nice if the change to `Clock::tick` is not described as a flaky test fix (although we can definitely reference the flaky test issue that motivated the change). 3rdparty/libprocess/src/clock.cpp (line 188) <https://reviews.apache.org/r/55576/#comment238112> Can you pull this out into a separate patch? 3rdparty/libprocess/src/tests/future_tests.cpp (lines 271 - 287) <https://reviews.apache.org/r/55576/#comment238111> Do we still need the additional scope here? 3rdparty/libprocess/src/tests/future_tests.cpp (line 283) <https://reviews.apache.org/r/55576/#comment238110> Hm.. why is this `Seconds(2)`? Shouldn't this just be `Milliseconds(1)`? - Benjamin Mahler On Feb. 20, 2017, 1:41 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55576/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2017, 1:41 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Hindman, and Joris Van > Remoortere. > > > Bugs: MESOS-6907 > https://issues.apache.org/jira/browse/MESOS-6907 > > > Repository: mesos > > > Description > ------- > > Fixed a flakiness in `FutureTest.After3` caused by the way > libprocess processes timers. > > Timers are not clear before setting `clock::settle` to > `false`, which make cause a race condition for callers which > expect timers' thunks to be destroyed after the timer is > called and the clock is settled. > > The issue is addressed by explicitly calling > `std::list<T>::clear()` on the `timedout` timer's list > once the timers are executed. > > > Diffs > ----- > > 3rdparty/libprocess/src/clock.cpp 6116c82377b1c2f33c5f4bc14890d35f34cc973d > 3rdparty/libprocess/src/tests/future_tests.cpp > 380755f9a2329d548969cfb2332c79aacbf7fff2 > > Diff: https://reviews.apache.org/r/55576/diff/ > > > Testing > ------- > > ```sh > # Put the machine under stress. > $ stress -c 4 -t 2600 -d 2 -i 2 & > $ ./${MESOS_SRC_ROOT}/build/3rdparty/libprocess/libprocess-tests \ > --gtest_filter="FutureTest.After3" --gtest_repeat=1000000 > --gtest_break_on_failure > $ ./${MESOS_SRC_ROOT}/build/build/src/mesos-tests > ``` > > > Thanks, > > Alexander Rojas > >
