----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51028/#review147697 -----------------------------------------------------------
Could you run the allocator tests with a high number of iterations (e.g., 100) to make sure no tests are flaky? src/tests/hierarchical_allocator_tests.cpp (lines 2718 - 2720) <https://reviews.apache.org/r/51028/#comment214910> How about: ``` // Wait for the allocation triggered from `addSlave()` to complete. // Otherwise `addFramework()` below may not trigger a new allocation // because the allocator batches them. ``` I reordered words in the first sentence and clarified `close succession` in the second. src/tests/hierarchical_allocator_tests.cpp (lines 2774 - 2779) <https://reviews.apache.org/r/51028/#comment215028> To really measure time, here we shouldn't pause clock and settle. To make sure there are two allocations without settling the clock, how about we add two frameworks and use ``` // Wait for the allocation to complete. AWAIT_READY(allocations.get()); ``` after each? src/tests/hierarchical_allocator_tests.cpp (lines 3123 - 3125) <https://reviews.apache.org/r/51028/#comment214964> After reading this comment people could still wonder why this is important because we are not measuring how many allocations we did here so why does it matter? I suggest something along the lines of ``` // Wait for the allocation triggered from `addFramework(framework1)` // to complete. Otherwise due to a race between `addFramework(framework2)` // and the next allocation (because it's run asynchronously), framework2 // may or may not be allocated resources. For simplicity here we give // all resources to framework1 as all we wanted to achieve in this step // is to recover all resources to set up the allocator for the next batch // allocation. ``` - Jiang Yan Xu On Aug. 23, 2016, 1:47 a.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51028/ > ----------------------------------------------------------- > > (Updated Aug. 23, 2016, 1:47 a.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-3157 > https://issues.apache.org/jira/browse/MESOS-3157 > > > Repository: mesos > > > Description > ------- > > - Per MESOS-3157, if cluster events with the possibility > of triggering an allocation occur rapidly and test > assertions depend on gleaning information from assumed > order, it is likely they will fail due to lack of parity > between event and actual allocation. Settle the clock > between these events in tests sensitive to this to ensure > expected allocations occur. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > cbed333f497016fe2811f755028796012b41db77 > > Diff: https://reviews.apache.org/r/51028/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jacob Janco > >
