----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41986/#review113332 -----------------------------------------------------------
Ship it! Thanks for triaging this! - Jie Yu On Jan. 6, 2016, 5:54 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41986/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2016, 5:54 p.m.) > > > Review request for mesos, Alexander Rukletsov, Jie Yu, and Michael Park. > > > Bugs: MESOS-4208 > https://issues.apache.org/jira/browse/MESOS-4208 > > > Repository: mesos > > > Description > ------- > > Fixed race in persistent volume tests. This race occurred because > `scheduelerDriver.reviveOffers` will trigger an offer immediately, while the > tests originally assumed that settling and advancing the clock after reviving > was necessary to produce an offer. > > > Diffs > ----- > > src/tests/persistent_volume_tests.cpp > 2fb57814b2805bc76981d1877603a1a033f29289 > > Diff: https://reviews.apache.org/r/41986/diff/ > > > Testing > ------- > > In order to reproduce this bug, insert a short sleep onto line 1204 or line > 1402 of `src/tests/persistent_volume_tests.cpp` - note that these line > numbers refer to the file *before* applying this patch. > > The race is due to the fact that calling `reviveOffers` triggers a new offer, > while the code is assuming that the clock must be settled and advanced before > an offer will be sent. Thus, the offer may arrive before the `EXPECT_CALL` is > executed, causing a previous `EXPECT_CALL` to be triggered more times than > expected. > > By calling `EXPECT_CALL` before `reviveOffers`, this race is avoided. The > unnecessary `Clock:settle` and `Clock::advance` calls have been removed as > well. > > To test, > `GTEST_FILTER="PersistentVolumeTest.BadACLDropCreateAndDestroy:PersistentVolumeTest.BadACLNoPrincipal" > bin/mesos-tests.sh` was run both with and without `sleep(1);` inserted > before the relevant `EXPECT_CALL`s. > > > Thanks, > > Greg Mann > >
