-----------------------------------------------------------
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
> 
>

Reply via email to