----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49617/#review147911 -----------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp (lines 3977 - 3982) <https://reviews.apache.org/r/49617/#comment215217> Let's use the `Allocation` defined [here](https://github.com/apache/mesos/blob/9c6097f063405279efc07eec22457c2059653f07/src/tests/hierarchical_allocator_tests.cpp#L79-L83) for better consistency in the code. We haven't been doing this consistently but we can start making it more consistent. (More context in /r/50868/) src/tests/hierarchical_allocator_tests.cpp (line 3984) <https://reviews.apache.org/r/49617/#comment215302> s/allocations/frameworkAllocations/ Sorry I know you were using the examples from SuppressOffer but we are just starting to clean up. The name `allocations` also refers to a class member so here `vector<Allocation> allocations;` is basically shadowing it. To avoid confusion I think it's best to avoid variable shadowing. src/tests/hierarchical_allocator_tests.cpp (line 4051) <https://reviews.apache.org/r/49617/#comment215653> IIRC your code used to suppress first but we don't *suppress* anymore so let's update the comment. Plus I think it'll be more clear to have some high level comments. We are simulating a "failover" so let's describe how what we are doing here constitute steps in the failover. i.e., here: ``` // 1. Disconnect all frameworks to simulate a failover. ``` Below (right above `size_t allocationsCount = 5;`) ``` // 2. Frameworks reconnect in the failover. Master processes them in batches. ``` src/tests/hierarchical_allocator_tests.cpp (line 4065) <https://reviews.apache.org/r/49617/#comment215651> Move settle to above the loop. When allocations triggered by `deactivateFramework` are running, the `foreach` on `allocations` doesn't work. `recoverResources` doesn't trigger allocations (yet) so we don't need to settle after the loop. This may change but let's not worry about it for now. src/tests/hierarchical_allocator_tests.cpp (line 4083) <https://reviews.apache.org/r/49617/#comment215646> Why suppress offers? Not all frameworks do this. I suggested some comments in an earlier revision: ``` This is to simulate the behavior of non-greedy frameworks: after the failover they immediately decline and suppress offers until further work is requested. ``` src/tests/hierarchical_allocator_tests.cpp (line 4085) <https://reviews.apache.org/r/49617/#comment215648> s/thus a separate loop/thus a separate loop to simulate it/ src/tests/hierarchical_allocator_tests.cpp (line 4101) <https://reviews.apache.org/r/49617/#comment215638> Indeed. Per Guangya's comment, we actually need to move this up to above the `recoverResources()` loop. When allocations triggered by `activateFramework` and `suppressOffers` are running, the `foreach` on `allocations` doesn't work. `recoverResources` doesn't trigger allocations (yet) so we don't need to settle after the loop. This may change but let's not worry about it for now. src/tests/hierarchical_allocator_tests.cpp (line 4109) <https://reviews.apache.org/r/49617/#comment215642> As discussed, here let's print out the number of actual allocation runs. Also s/allocator/Allocator/ As Guangya suggested, we can add something like "with 5000 frameworks failed over and suppressed offers" - Jiang Yan Xu On Aug. 16, 2016, 7:26 p.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49617/ > ----------------------------------------------------------- > > (Updated Aug. 16, 2016, 7:26 p.m.) > > > Review request for mesos, Joris Van Remoortere and Jiang Yan Xu. > > > Bugs: MESOS-5780 > https://issues.apache.org/jira/browse/MESOS-5780 > > > Repository: mesos > > > Description > ------- > > - This benchmark measures latency to stability of > the allocator following disconnection and > reconnection of all frameworks. > - In this scenario, frameworks are offered resources > and suppressed in batches. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > cbed333f497016fe2811f755028796012b41db77 > > Diff: https://reviews.apache.org/r/49617/diff/ > > > Testing > ------- > > MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.FrameworkFailover*" make check > > Sample Output: > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23 > Using 10000 agents and 6000 frameworks > Added 6000 frameworks in 113410us > Added 10000 agents in 6.83980663333333mins > allocator settled after 3.28683733333333mins > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23 > (609255 ms)[ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24 > Using 20000 agents and 1 frameworks > Added 1 frameworks in 190us > Added 20000 agents in 4.752954secs > allocator settled after 7us > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24 > (6332 ms) > > > Thanks, > > Jacob Janco > >