Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review167382
---



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On Aug. 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated Aug. 17, 2016, 2:26 a.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/7/
> 
> 
> Testing
> ---
> 
> MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.FrameworkFailover*" make check
> 
> Sample Output:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
> Using 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

> On 八月 24, 2016, 9:12 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 4091-4099
> > 
> >
> > Can we make sure all of the `allocations` are available when 
> > `recoverResources`? Do we need a `Clock::settle();` after all 
> > `suppressOffers` finshed in line 4090?

Looking forward to the updated patch, just want to highlight that adding 
`Clock::settle` after `activateFramework` maybe better as we can make sure the 
all of `allocate()` operations are finshed before suppress offer. 

The logic could be 
```
activateFramework()

Clock::settle()

suppressOffers()

recoverResources()

Clock::settle()
```


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review146625
---


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

> On 九月 9, 2016, 7:06 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4035
> > 
> >
> > You can call ` SlaveInfo createSlaveInfo(const Resources& resources)` 
> > so as to avoid calling `Resources::parse()` each time.
> 
> Jacob Janco wrote:
> We want a unique slaveId for each per: `slaveId.set_value("agent" + 
> stringify(nextSlaveId++));` in createSlaveInfo

I think that the API `SlaveInfo createSlaveInfo(const Resources& resources)` 
can also keep a unique slaveId for each agent here 
https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L239-L249
 , comments?


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review148311
---


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

> On 九月 8, 2016, 5:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4065
> > 
> >
> > 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.
> 
> Guangya Liu wrote:
> Just FYI, I'm now trying to enable `recoverResources` trigger an 
> allocation here https://reviews.apache.org/r/51621/
> 
> Jacob Janco wrote:
> Yan, pending that patch, perhaps leave the settle where it is?

@Jiang Yan, I think that the `deactivateFramework` will not trigger allocation, 
so seems it is OK to leave the `settle` here?


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review147911
---


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

> On Sept. 9, 2016, 7:06 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4035
> > 
> >
> > You can call ` SlaveInfo createSlaveInfo(const Resources& resources)` 
> > so as to avoid calling `Resources::parse()` each time.

We want a unique slaveId for each per: `slaveId.set_value("agent" + 
stringify(nextSlaveId++));` in createSlaveInfo


- Jacob


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review148311
---


On Aug. 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated Aug. 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

> On Sept. 8, 2016, 5:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4065
> > 
> >
> > 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.
> 
> Guangya Liu wrote:
> Just FYI, I'm now trying to enable `recoverResources` trigger an 
> allocation here https://reviews.apache.org/r/51621/

Yan, pending that patch, perhaps leave the settle where it is?


- Jacob


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review147911
---


On Aug. 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated Aug. 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

> On 九月 8, 2016, 5:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4065
> > 
> >
> > 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.

Just FYI, I'm now trying to enable `recoverResources` trigger an allocation 
here https://reviews.apache.org/r/51621/


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review147911
---


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review148311
---




src/tests/hierarchical_allocator_tests.cpp (line 4035)


You can call ` SlaveInfo createSlaveInfo(const Resources& resources)` so as 
to avoid calling `Resources::parse()` each time.


- Guangya Liu


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

> On 八月 24, 2016, 9:12 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4030
> > 
> >
> > How about adding `ports` resources to the allocation here.
> > 
> > ```
> > // Each agent has a portion of it's resources allocated to a single
> > // framework. We round-robin through the frameworks when allocating.
> > Resources allocation = 
> > Resources::parse("cpus:16;mem:1024;disk:1024").get();
> > 
> > Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 
> > 32000), 16);
> > ASSERT_SOME(ranges);
> > ASSERT_EQ(16, ranges->range_size());
> > 
> > allocation += createPorts(ranges.get());
> > ```
> 
> Jacob Janco wrote:
> I took this out to simplify the test.
> 
> Guangya Liu wrote:
> I think that we should simulate a full allocate scernario in the 
> benchmark test to include `ports` as well just like other benchmark test, as 
> we actually have some special logic to handle non-scalar resources in sorter 
> when allocate resources, comments?
> 
> Jiang Yan Xu wrote:
> What's the special logic that you were referring to? I am not aganist 
> adding to ports here but once this test is committed let's do some refactor 
> to pull common elements into helpers or member variables so new benchmark 
> tests don't need to copy a lot of code.

+1 to add some common helpers, I was proposing adding `ports` here is mainly 
because now all benchmark test include `ports` resources and also the `sorter` 
have some logic for non-scalar resources, such as 
`createStrippedScalarQuantity` etc which will filter out the non-scalar 
resources, so adding `ports` here would simulate a real use cases here.


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review146625
---


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jiang Yan Xu]

> On Aug. 24, 2016, 2:12 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4030
> > 
> >
> > How about adding `ports` resources to the allocation here.
> > 
> > ```
> > // Each agent has a portion of it's resources allocated to a single
> > // framework. We round-robin through the frameworks when allocating.
> > Resources allocation = 
> > Resources::parse("cpus:16;mem:1024;disk:1024").get();
> > 
> > Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 
> > 32000), 16);
> > ASSERT_SOME(ranges);
> > ASSERT_EQ(16, ranges->range_size());
> > 
> > allocation += createPorts(ranges.get());
> > ```
> 
> Jacob Janco wrote:
> I took this out to simplify the test.
> 
> Guangya Liu wrote:
> I think that we should simulate a full allocate scernario in the 
> benchmark test to include `ports` as well just like other benchmark test, as 
> we actually have some special logic to handle non-scalar resources in sorter 
> when allocate resources, comments?

What's the special logic that you were referring to? I am not aganist adding to 
ports here but once this test is committed let's do some refactor to pull 
common elements into helpers or member variables so new benchmark tests don't 
need to copy a lot of code.


- Jiang Yan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review146625
---


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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jiang Yan Xu]

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


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)


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


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)


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)


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)


s/thus a separate loop/thus a separate loop to simulate it/



src/tests/hierarchical_allocator_tests.cpp (line 4101)


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)


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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [  

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

> On 八月 24, 2016, 9:12 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4030
> > 
> >
> > How about adding `ports` resources to the allocation here.
> > 
> > ```
> > // Each agent has a portion of it's resources allocated to a single
> > // framework. We round-robin through the frameworks when allocating.
> > Resources allocation = 
> > Resources::parse("cpus:16;mem:1024;disk:1024").get();
> > 
> > Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 
> > 32000), 16);
> > ASSERT_SOME(ranges);
> > ASSERT_EQ(16, ranges->range_size());
> > 
> > allocation += createPorts(ranges.get());
> > ```
> 
> Jacob Janco wrote:
> I took this out to simplify the test.

I think that we should simulate a full allocate scernario in the benchmark test 
to include `ports` as well just like other benchmark test, as we actually have 
some special logic to handle non-scalar resources in sorter when allocate 
resources, comments?


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review146625
---


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

> On Aug. 24, 2016, 9:12 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4030
> > 
> >
> > How about adding `ports` resources to the allocation here.
> > 
> > ```
> > // Each agent has a portion of it's resources allocated to a single
> > // framework. We round-robin through the frameworks when allocating.
> > Resources allocation = 
> > Resources::parse("cpus:16;mem:1024;disk:1024").get();
> > 
> > Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 
> > 32000), 16);
> > ASSERT_SOME(ranges);
> > ASSERT_EQ(16, ranges->range_size());
> > 
> > allocation += createPorts(ranges.get());
> > ```

I took this out to simplify the test.


- Jacob


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review146625
---


On Aug. 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated Aug. 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review146625
---




src/tests/hierarchical_allocator_tests.cpp (line 4030)


How about adding `ports` resources to the allocation here.

```
// Each agent has a portion of it's resources allocated to a single
// framework. We round-robin through the frameworks when allocating.
Resources allocation = Resources::parse("cpus:16;mem:1024;disk:1024").get();

Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 32000), 
16);
ASSERT_SOME(ranges);
ASSERT_EQ(16, ranges->range_size());

allocation += createPorts(ranges.get());
```



src/tests/hierarchical_allocator_tests.cpp (line 4035)


Add `ports` resource here for each agent.



src/tests/hierarchical_allocator_tests.cpp (lines 4091 - 4099)


Can we make sure all of the `allocations` are available when 
`recoverResources`? Do we need a `Clock::settle();` after all `suppressOffers` 
finshed in line 4090?


- Guangya Liu


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

> On 八月 18, 2016, 2:31 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 4109
> > 
> >
> > What about update the message as
> > 
> > ```
> > cout << "allocator settled after " << watch.elapsed()
> >  << " with activate/suppress " << addFrameworkCount << endl;
> > ```

Lost one word in above comment:

```
cout << "allocator settled after " << watch.elapsed()
 << " to activate/suppress " << addFrameworkCount
 << " frameworks" << endl;
```


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review146059
---


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review146059
---




src/tests/hierarchical_allocator_tests.cpp (lines 4003 - 4004)


You can simplify this as following:

```
initialize(master::Flags(), offerCallback);
```



src/tests/hierarchical_allocator_tests.cpp (line 4035)


Just FYI: When create agent, the API `createSlaveInfo` need to parse the 
resources in every loop, this is not good to evaluate the add agent 
performance, I created a patch here to remove the logic of parse resources in 
`createSlaveInfo`

https://reviews.apache.org/r/50693/



src/tests/hierarchical_allocator_tests.cpp (lines 4045 - 4046)


new line here



src/tests/hierarchical_allocator_tests.cpp (line 4051)


What about update the comments as

```
// Deactivates the framework to make sure it will not got resources any 
more. 
```



src/tests/hierarchical_allocator_tests.cpp (lines 4051 - 4065)


What about adding time elapse here for `deactivateFramework`? The time 
elapse should include both `deactivateFramework` and `recoverResources`.



src/tests/hierarchical_allocator_tests.cpp (line 4106)


I think we can remove this?



src/tests/hierarchical_allocator_tests.cpp (line 4109)


What about update the message as

```
cout << "allocator settled after " << watch.elapsed()
 << " with activate/suppress " << addFrameworkCount << endl;
```


- Guangya Liu


On 八月 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated 八月 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review145976
---



Patch looks great!

Reviews applied: [49617]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Aug. 17, 2016, 2:26 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated Aug. 17, 2016, 2:26 a.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 1 agents and 6000 frameworks
> Added 6000 frameworks in 113410us
> Added 1 agents in 6.8398066333mins
> allocator settled after  3.286837mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
>  (609255 ms)[ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
> Using 2 agents and 1 frameworks
> Added 1 frameworks in 190us
> Added 2 agents in 4.752954secs
> allocator settled after  7us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
>  (6332 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/
---

(Updated Aug. 17, 2016, 2:26 a.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 (updated)
-

  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 1 agents and 6000 frameworks
Added 6000 frameworks in 113410us
Added 1 agents in 6.8398066333mins
allocator settled after  3.286837mins
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
 (609255 ms)[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
Using 2 agents and 1 frameworks
Added 1 frameworks in 190us
Added 2 agents in 4.752954secs
allocator settled after  7us
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
 (6332 ms)


Thanks,

Jacob Janco

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/
---

(Updated Aug. 17, 2016, 2:17 a.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 (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
cbed333f497016fe2811f755028796012b41db77 

Diff: https://reviews.apache.org/r/49617/diff/


Testing (updated)
---

MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.FrameworkFailover*" make check

Sample Output:
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
Using 1 agents and 6000 frameworks
Added 6000 frameworks in 113410us
Added 1 agents in 6.8398066333mins
allocator settled after  3.286837mins
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/23
 (609255 ms)[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
Using 2 agents and 1 frameworks
Added 1 frameworks in 190us
Added 2 agents in 4.752954secs
allocator settled after  7us
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/24
 (6332 ms)


Thanks,

Jacob Janco

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review142842
---



Patch looks great!

Reviews applied: [49617]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 8, 2016, 3:11 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated July 8, 2016, 3:11 a.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 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jiang Yan Xu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review142764
---




src/tests/hierarchical_allocator_tests.cpp (line 3724)


Ditto. Unnecessary `i < frameworkCount`.


- Jiang Yan Xu


On July 7, 2016, 8:11 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated July 7, 2016, 8:11 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 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jiang Yan Xu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review142075
---




src/tests/hierarchical_allocator_tests.cpp (line 3690)


A blank line above.



src/tests/hierarchical_allocator_tests.cpp (line 3706)


A blank line above.



src/tests/hierarchical_allocator_tests.cpp (lines 3709 - 3710)


Empty lines above and below.



src/tests/hierarchical_allocator_tests.cpp (line 3717)


The std::min above already makes sure batchEnd is at most frameworkCount so 
we don't need to worry about i < frameworkCount here.



src/tests/hierarchical_allocator_tests.cpp (lines 3721 - 3723)


Add to the end: `, thus a separate loop.` so it's more clear?


- Jiang Yan Xu


On July 7, 2016, 8:11 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated July 7, 2016, 8:11 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 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/
---

(Updated July 8, 2016, 3:11 a.m.)


Review request for mesos, Joris Van Remoortere and Jiang Yan Xu.


Changes
---

Fix loop condition.


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 (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

Diff: https://reviews.apache.org/r/49617/diff/


Testing
---

make check


Thanks,

Jacob Janco

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/
---

(Updated July 8, 2016, 2:07 a.m.)


Review request for mesos, Joris Van Remoortere and Jiang Yan Xu.


Changes
---

Get rid of offerCount


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 (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

Diff: https://reviews.apache.org/r/49617/diff/


Testing
---

make check


Thanks,

Jacob Janco

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

> On July 6, 2016, 6:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 3711-3725
> > 
> >
> > 1. We should recover resources after the whole batch is finished and 
> > not after each `activateFramework` is called, as this is often the case in 
> > realistic scenarios.
> > 2. If we don't `settle()` then vector is actually not thread-safe so I 
> > think we need to settle here.
> > 
> > How about this for the entire loop?
> > 
> > ```
> >   for (unsigned batchStart = 0;
> >batchStart < frameworkCount;
> >batchStart += batchSize) {
> > unsigned batchEnd = min(batchStart + batchSize, frameworkCount);
> >   
> > // Activate all frameworks in this batch to trigger allocations.
> > for (unsigned i = batchStart; i < batchEnd; i++) {
> >   allocator->activateFramework(frameworks[i].id());
> > }
> > 
> > // Suppress offers in this batch after all activations. This is
> > // requires another message round trip between schedulers and the
> > // master following the activation.
> > for (unsigned i = batchStart; i < batchEnd; i++) {
> >   allocator->suppressOffers(frameworks[k].id());
> > }
> > 
> > // Make sure all resources are offered. This avoids data races in
> > // the offers vector.
> > Clock::settle();
> > 
> > foreach (auto offer, offers) {
> >   allocator->recoverResources(
> >   offer.frameworkId, 
> >   offer.slaveId,
> >   offer.resources, 
> >   None());
> > }
> > 
> > offers.clear();
> >   }
> > ```
> 
> Jacob Janco wrote:
> added a termination condition  for (unsigned i = batchStart; i < batchEnd 
> && i < frameworkCount; i++) {
> 
> to account for all frameworkCount cases. 
> 
> Also, I recover offers there because only framework 1 of the batch will 
> get an offer, so we will get 1005 offers for the whole of a test run with 
> 1000 agents the 5 being the 5 batches of activate/suppress. What do you think?
> 
> Jacob Janco wrote:
> Actually strike that wasn't thinking, I'll report offers.size() at the 
> end! agree with this completely!

This seems irrelevant to the test, I'll remove that metric entirely.


- Jacob


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review140887
---


On July 8, 2016, 1:22 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated July 8, 2016, 1:22 a.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 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

> On July 6, 2016, 6:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 3711-3725
> > 
> >
> > 1. We should recover resources after the whole batch is finished and 
> > not after each `activateFramework` is called, as this is often the case in 
> > realistic scenarios.
> > 2. If we don't `settle()` then vector is actually not thread-safe so I 
> > think we need to settle here.
> > 
> > How about this for the entire loop?
> > 
> > ```
> >   for (unsigned batchStart = 0;
> >batchStart < frameworkCount;
> >batchStart += batchSize) {
> > unsigned batchEnd = min(batchStart + batchSize, frameworkCount);
> >   
> > // Activate all frameworks in this batch to trigger allocations.
> > for (unsigned i = batchStart; i < batchEnd; i++) {
> >   allocator->activateFramework(frameworks[i].id());
> > }
> > 
> > // Suppress offers in this batch after all activations. This is
> > // requires another message round trip between schedulers and the
> > // master following the activation.
> > for (unsigned i = batchStart; i < batchEnd; i++) {
> >   allocator->suppressOffers(frameworks[k].id());
> > }
> > 
> > // Make sure all resources are offered. This avoids data races in
> > // the offers vector.
> > Clock::settle();
> > 
> > foreach (auto offer, offers) {
> >   allocator->recoverResources(
> >   offer.frameworkId, 
> >   offer.slaveId,
> >   offer.resources, 
> >   None());
> > }
> > 
> > offers.clear();
> >   }
> > ```
> 
> Jacob Janco wrote:
> added a termination condition  for (unsigned i = batchStart; i < batchEnd 
> && i < frameworkCount; i++) {
> 
> to account for all frameworkCount cases. 
> 
> Also, I recover offers there because only framework 1 of the batch will 
> get an offer, so we will get 1005 offers for the whole of a test run with 
> 1000 agents the 5 being the 5 batches of activate/suppress. What do you think?

Actually strike that wasn't thinking, I'll report offers.size() at the end! 
agree with this completely!


- Jacob


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review140887
---


On July 8, 2016, 1:22 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated July 8, 2016, 1:22 a.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 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/
---

(Updated July 8, 2016, 1:22 a.m.)


Review request for mesos, Joris Van Remoortere and Jiang Yan Xu.


Changes
---

Fixed a few typos and spacing.


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 (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

Diff: https://reviews.apache.org/r/49617/diff/


Testing
---

make check


Thanks,

Jacob Janco

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

> On July 6, 2016, 6:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 3629
> > 
> >
> > Looks like our test doesn't use `offerCount` at all?
> > 
> > If we keep `offerCount` we can log how many total offers are generated 
> > at the end of the test. It's not an essential metric but I guess it would 
> > be nice to confirm.

Yes this is why I kept it here, I was using it while debugging. I'll add it as 
a metric at the end.


> On July 6, 2016, 6:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 3704
> > 
> >
> > Use `std::lround`?
> > 
> > This math here would be equivalent but it would be nice if we don't 
> > have to explain it. :)

So lround rounds down I want to do a std::ceil without having to cast the 
double. For instance 103 frameworks, I want batches of 21 rather than 20. Also 
for the case of frameworkCount=1 lround will give me batch size 0 and I get an 
infinite loop.


> On July 6, 2016, 6:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 3708
> > 
> >
> > Single space around `+=`.
> > 
> > More importantly, will this exclude the frameworks numbered after the 
> > last "full-sized" batch? i.e., framework 5-8 if frameworkCount == 9?
> > 
> > I suggest we restructure the loop as described below.

I overlooked this, but we will want:

for (unsigned i = batchStart; i < batchEnd && i < frameworkCount; i++) with the 
above comment for the ceiling. so in the case of 103 frameworks I hit 84 with a 
batchend of 105 and I'll terminate both inner loops at i < 103.


> On July 6, 2016, 6:24 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 3711-3725
> > 
> >
> > 1. We should recover resources after the whole batch is finished and 
> > not after each `activateFramework` is called, as this is often the case in 
> > realistic scenarios.
> > 2. If we don't `settle()` then vector is actually not thread-safe so I 
> > think we need to settle here.
> > 
> > How about this for the entire loop?
> > 
> > ```
> >   for (unsigned batchStart = 0;
> >batchStart < frameworkCount;
> >batchStart += batchSize) {
> > unsigned batchEnd = min(batchStart + batchSize, frameworkCount);
> >   
> > // Activate all frameworks in this batch to trigger allocations.
> > for (unsigned i = batchStart; i < batchEnd; i++) {
> >   allocator->activateFramework(frameworks[i].id());
> > }
> > 
> > // Suppress offers in this batch after all activations. This is
> > // requires another message round trip between schedulers and the
> > // master following the activation.
> > for (unsigned i = batchStart; i < batchEnd; i++) {
> >   allocator->suppressOffers(frameworks[k].id());
> > }
> > 
> > // Make sure all resources are offered. This avoids data races in
> > // the offers vector.
> > Clock::settle();
> > 
> > foreach (auto offer, offers) {
> >   allocator->recoverResources(
> >   offer.frameworkId, 
> >   offer.slaveId,
> >   offer.resources, 
> >   None());
> > }
> > 
> > offers.clear();
> >   }
> > ```

added a termination condition  for (unsigned i = batchStart; i < batchEnd && i 
< frameworkCount; i++) {

to account for all frameworkCount cases. 

Also, I recover offers there because only framework 1 of the batch will get an 
offer, so we will get 1005 offers for the whole of a test run with 1000 agents 
the 5 being the 5 batches of activate/suppress. What do you think?


- Jacob


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review140887
---


On July 8, 2016, 1:06 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated July 8, 2016, 1:06 a.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 

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/
---

(Updated July 8, 2016, 1:06 a.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 (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

Diff: https://reviews.apache.org/r/49617/diff/


Testing
---

make check


Thanks,

Jacob Janco

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jiang Yan Xu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review140887
---




src/tests/hierarchical_allocator_tests.cpp (line 3629)


Looks like our test doesn't use `offerCount` at all?

If we keep `offerCount` we can log how many total offers are generated at 
the end of the test. It's not an essential metric but I guess it would be nice 
to confirm.



src/tests/hierarchical_allocator_tests.cpp (line 3671)


Strictly speaking this test doesn't need these used resources so we should 
explain a little bit.

```
// Add some used resources on each agent so it's more realistic in a 
framework failover scenario.
```



src/tests/hierarchical_allocator_tests.cpp (line 3677)


Someone sometime later still needs to do a sweep of the codebase for this 
but now that we are 1.0 let's start using the new terminology in places that 
are not confusing.

s/slave/agent/.



src/tests/hierarchical_allocator_tests.cpp (lines 3680 - 3685)


Move this below framework deactivation and add a comment:

```
// During framework failover all its offers are recovered.
```



src/tests/hierarchical_allocator_tests.cpp (lines 3682 - 3683)


We don't use this indentation format per the style guide.

How about just:

```
allocator->recoverResources(
offer.frameworkId,
offer.slaveId,
offer.resources,
None());
```



src/tests/hierarchical_allocator_tests.cpp (line 3685)


Use a blank line above.



src/tests/hierarchical_allocator_tests.cpp (line 3686)


No reset.



src/tests/hierarchical_allocator_tests.cpp (lines 3690 - 3691)


`deactivateFramework` would reset the effect of `suppressOffers` so this 
line is unnecessary.



src/tests/hierarchical_allocator_tests.cpp (line 3690)


No need to suppress here as we are failing over anyways.



src/tests/hierarchical_allocator_tests.cpp (line 3700)


s/resoures/resources/



src/tests/hierarchical_allocator_tests.cpp (lines 3700 - 3702)


Add:

This is to simulate the real world scenario where a large number of 
frameworks fail over simulatenously but the allocator processes framework 
activations in batches interleaved by offer declinations and suppressions from 
frameworks reregistered earlier.



src/tests/hierarchical_allocator_tests.cpp (line 3703)


```
TODO(jjanco): Parameterize the test by the batch size instead of using an 
arbitrary number.
```



src/tests/hierarchical_allocator_tests.cpp (line 3704)


Use `std::lround`?

This math here would be equivalent but it would be nice if we don't have to 
explain it. :)



src/tests/hierarchical_allocator_tests.cpp (line 3708)


Single space around `+=`.

More importantly, will this exclude the frameworks numbered after the last 
"full-sized" batch? i.e., framework 5-8 if frameworkCount == 9?

I suggest we restructure the loop as described below.



src/tests/hierarchical_allocator_tests.cpp (lines 3709 - 3710)


Add:

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 (lines 3711 - 3725)


1. We should recover resources after the whole batch is finished and not 
after each `activateFramework` is called, as this is often the case in 
realistic scenarios.
2. If we don't `settle()` then vector is actually not thread-safe so I 
think we need to settle here.

How about this for the entire loop?

```
  for (unsigned batchStart = 0;
   batchStart < frameworkCount;
   batchStart += batchSize) {
unsigned batchEnd = min(batchStart + batchSize, frameworkCount);
  
// Activate all frameworks in this batch to trigger allocations.
for (unsigned i = batchStart; i < batchEnd; i++) {
  allocator->activateFramework(frameworks[i].id());
}

// 

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/#review140718
---



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On July 4, 2016, 11:04 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated July 4, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> 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 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/
---

(Updated July 4, 2016, 11:04 p.m.)


Review request for mesos.


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

Diff: https://reviews.apache.org/r/49617/diff/


Testing
---

make check


Thanks,

Jacob Janco

Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49617/
---

Review request for mesos.


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

Diff: https://reviews.apache.org/r/49617/diff/


Testing
---

make check


Thanks,

Jacob Janco