Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50868/#review167380 --- 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. 5, 2016, 9:50 p.m., Guangya Liu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50868/ > --- > > (Updated Aug. 5, 2016, 9:50 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > --- > > The current benchmark test of `SuppressOffers` is using `Allocation` > to specify the resources from an `OfferCallback`, but here seems > using `OfferedResources` maybe better, as here we are actually > constructing the offers on each agent, but the `Allocation` is an > allocator concept and it means the resources allocated for a specified > framework. > > > Diffs > - > > src/tests/hierarchical_allocator_tests.cpp > cbed333f497016fe2811f755028796012b41db77 > > > Diff: https://reviews.apache.org/r/50868/diff/1/ > > > Testing > --- > > make > make check > > ``` > ./bin/mesos-tests.sh --benchmark > --gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0" > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 > Using 1000 agents and 1 frameworks > Added 1 frameworks in 841us > Added 1000 agents in 480991us > allocate() took 264196us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 270110us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 270098us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 264314us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 268732us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 > (4511 ms) > [--] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test (4511 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (4522 ms total) > [ PASSED ] 1 test. > ``` > > > Thanks, > > Guangya Liu > >
Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
> On 八月 25, 2016, 10:56 p.m., Jiang Yan Xu wrote:
> > I don't know. Making such distinction feels like splitting hairs to me.
> > Yeah there are subtle differences between the two concepts but to me:
> >
> > - `offerCallback` may not be the best name what we could have come up with
> > even though in `Master::offer()` we do construct offers.
> > - When offer objects are constructed, we call the resources in them
> > "offered resources".
> > - When we parse resources from offer objects, we call them "offered
> > resources".
> > - In this test, namely, the "Allocator" tests, we are indeed only
> > allocating resources. (I don't see "Offer" objects being created).
> >
> > Later if the allocator actually hands out offer objects, then we probably
> > need to change the tests to use offers directly but right now I think 1)
> > "allocation" is most correct, 2) it's not a big deal if people treat them
> > as synonymous terms.
> >
> > That said, I think reusing the "Allocation" defined at the top of the test
> > makes sense.
>
> Guangya Liu wrote:
> Thanks Jiang Yan, there are acutally some discussion here
> https://reviews.apache.org/r/50677/ , the reason that I want to update this
> is because:
>
> 1) Make the code consistent, in the allocator test cases, some benchmark
> test using `OfferedResources` while some using `Allocation`, when people want
> to add new benchmark test, they will be confused: Which one shall I use,
> `OfferedResources` or `Allocation`, so here we need to make the code
> consistent with each other for better reading, easy to maintain and
> consistent.
>
> 2) Seems here using `OfferedResources` maybe better, as here we are
> actually constructing the `offers` on each agent; But the `Allocation` is an
> allocator concept and it means the resources for a specified framework. Also
> all of the log messages are highlighting `offer` in benchmark test such as
> here
> https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3885
>
> 3) The benchmark test is focusing on `offers` even though the allocator
> know nothing about `offer`, but here we are just simulating `offer
> operations` in benchmark test. For other unit test cases other than benchmark
> test, they are focusing on `allocations` for each framework, so it is using
> `Allocation`.
>
> What do you think?
>
> Jiang Yan Xu wrote:
> Thanks for the reply. I've listed a few critia that I think could explain
> their subtle differences but I try to see if making such a distinction leads
> to simplity and clarity of the code. i.e., would everyone agree with the way
> we make such distinction and would it help readers understand the code
> better. If it's controversial, then perhaps we should aim for consistently. :)
>
> In terms of code in this file, yes I agree there's inconsistency and we
> should address it. To that end I think:
>
> To 1): The question to me is to choose between *OfferedResources ->
> Allocation* or *Allocation -> OfferedResources*. I am leaning towards
> *OfferedResources -> Allocation* and FWIW `Allocation` is a more established
> usage than `OfferedResources` in this file.
>
> To 2) and 3): sorry I failed to see the fundamental difference between
> the benchmarks and the unit tests here in terms of `offers` vs `allocations`.
> The unit tests, espcially later-added ones, follow basically the same pattern
> as the benchmarks. The benchmarks are just different in the use of
> expectations vs. measurements and the number of iterations. It's not
> convincing to me that in some cases we need to call them OfferedResources and
> in others we should call them Allocations.
>
> So overall I think it's better to change to use the Allocation struct
> defined at the top to achieve consistency. It looks to me that all
> occurrences of `offers` in the log messages can be changed to `allocations`
> as well.
>
> Let me know what you think.
>
> Guangya Liu wrote:
> Hi Jiang Yan, are you mentioning the `Allocation` definition here
> https://github.com/apache/mesos/blob/9c6097f063405279efc07eec22457c2059653f07/src/tests/hierarchical_allocator_tests.cpp#L79-L83
> ? They are a bit different:
>
> ```
> struct Allocation
> {
> FrameworkID frameworkId;
> hashmap resources;
> };
> ```
>
> ```
> struct Allocation
> {
> FrameworkID frameworkId;
> SlaveID slaveId;
> Resources resources;
> };
> ```
>
> The above two is a bit difference: The first `Allocation` is an
> allocation for a framework and the second `Allocation` is actullay an `offer`
> as one framework can have multiple such `Allocation`.
>
> So it may not accurate to use `allocations` in the benchmark test as they
> are actually `offers` for each framework, comments? Thanks.
>
>
Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
> On Aug. 25, 2016, 3:56 p.m., Jiang Yan Xu wrote:
> > I don't know. Making such distinction feels like splitting hairs to me.
> > Yeah there are subtle differences between the two concepts but to me:
> >
> > - `offerCallback` may not be the best name what we could have come up with
> > even though in `Master::offer()` we do construct offers.
> > - When offer objects are constructed, we call the resources in them
> > "offered resources".
> > - When we parse resources from offer objects, we call them "offered
> > resources".
> > - In this test, namely, the "Allocator" tests, we are indeed only
> > allocating resources. (I don't see "Offer" objects being created).
> >
> > Later if the allocator actually hands out offer objects, then we probably
> > need to change the tests to use offers directly but right now I think 1)
> > "allocation" is most correct, 2) it's not a big deal if people treat them
> > as synonymous terms.
> >
> > That said, I think reusing the "Allocation" defined at the top of the test
> > makes sense.
>
> Guangya Liu wrote:
> Thanks Jiang Yan, there are acutally some discussion here
> https://reviews.apache.org/r/50677/ , the reason that I want to update this
> is because:
>
> 1) Make the code consistent, in the allocator test cases, some benchmark
> test using `OfferedResources` while some using `Allocation`, when people want
> to add new benchmark test, they will be confused: Which one shall I use,
> `OfferedResources` or `Allocation`, so here we need to make the code
> consistent with each other for better reading, easy to maintain and
> consistent.
>
> 2) Seems here using `OfferedResources` maybe better, as here we are
> actually constructing the `offers` on each agent; But the `Allocation` is an
> allocator concept and it means the resources for a specified framework. Also
> all of the log messages are highlighting `offer` in benchmark test such as
> here
> https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3885
>
> 3) The benchmark test is focusing on `offers` even though the allocator
> know nothing about `offer`, but here we are just simulating `offer
> operations` in benchmark test. For other unit test cases other than benchmark
> test, they are focusing on `allocations` for each framework, so it is using
> `Allocation`.
>
> What do you think?
>
> Jiang Yan Xu wrote:
> Thanks for the reply. I've listed a few critia that I think could explain
> their subtle differences but I try to see if making such a distinction leads
> to simplity and clarity of the code. i.e., would everyone agree with the way
> we make such distinction and would it help readers understand the code
> better. If it's controversial, then perhaps we should aim for consistently. :)
>
> In terms of code in this file, yes I agree there's inconsistency and we
> should address it. To that end I think:
>
> To 1): The question to me is to choose between *OfferedResources ->
> Allocation* or *Allocation -> OfferedResources*. I am leaning towards
> *OfferedResources -> Allocation* and FWIW `Allocation` is a more established
> usage than `OfferedResources` in this file.
>
> To 2) and 3): sorry I failed to see the fundamental difference between
> the benchmarks and the unit tests here in terms of `offers` vs `allocations`.
> The unit tests, espcially later-added ones, follow basically the same pattern
> as the benchmarks. The benchmarks are just different in the use of
> expectations vs. measurements and the number of iterations. It's not
> convincing to me that in some cases we need to call them OfferedResources and
> in others we should call them Allocations.
>
> So overall I think it's better to change to use the Allocation struct
> defined at the top to achieve consistency. It looks to me that all
> occurrences of `offers` in the log messages can be changed to `allocations`
> as well.
>
> Let me know what you think.
>
> Guangya Liu wrote:
> Hi Jiang Yan, are you mentioning the `Allocation` definition here
> https://github.com/apache/mesos/blob/9c6097f063405279efc07eec22457c2059653f07/src/tests/hierarchical_allocator_tests.cpp#L79-L83
> ? They are a bit different:
>
> ```
> struct Allocation
> {
> FrameworkID frameworkId;
> hashmap resources;
> };
> ```
>
> ```
> struct Allocation
> {
> FrameworkID frameworkId;
> SlaveID slaveId;
> Resources resources;
> };
> ```
>
> The above two is a bit difference: The first `Allocation` is an
> allocation for a framework and the second `Allocation` is actullay an `offer`
> as one framework can have multiple such `Allocation`.
>
> So it may not accurate to use `allocations` in the benchmark test as they
> are actually `offers` for each framework, comments? Thanks.
Yes
Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
> On 八月 25, 2016, 10:56 p.m., Jiang Yan Xu wrote:
> > I don't know. Making such distinction feels like splitting hairs to me.
> > Yeah there are subtle differences between the two concepts but to me:
> >
> > - `offerCallback` may not be the best name what we could have come up with
> > even though in `Master::offer()` we do construct offers.
> > - When offer objects are constructed, we call the resources in them
> > "offered resources".
> > - When we parse resources from offer objects, we call them "offered
> > resources".
> > - In this test, namely, the "Allocator" tests, we are indeed only
> > allocating resources. (I don't see "Offer" objects being created).
> >
> > Later if the allocator actually hands out offer objects, then we probably
> > need to change the tests to use offers directly but right now I think 1)
> > "allocation" is most correct, 2) it's not a big deal if people treat them
> > as synonymous terms.
> >
> > That said, I think reusing the "Allocation" defined at the top of the test
> > makes sense.
>
> Guangya Liu wrote:
> Thanks Jiang Yan, there are acutally some discussion here
> https://reviews.apache.org/r/50677/ , the reason that I want to update this
> is because:
>
> 1) Make the code consistent, in the allocator test cases, some benchmark
> test using `OfferedResources` while some using `Allocation`, when people want
> to add new benchmark test, they will be confused: Which one shall I use,
> `OfferedResources` or `Allocation`, so here we need to make the code
> consistent with each other for better reading, easy to maintain and
> consistent.
>
> 2) Seems here using `OfferedResources` maybe better, as here we are
> actually constructing the `offers` on each agent; But the `Allocation` is an
> allocator concept and it means the resources for a specified framework. Also
> all of the log messages are highlighting `offer` in benchmark test such as
> here
> https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3885
>
> 3) The benchmark test is focusing on `offers` even though the allocator
> know nothing about `offer`, but here we are just simulating `offer
> operations` in benchmark test. For other unit test cases other than benchmark
> test, they are focusing on `allocations` for each framework, so it is using
> `Allocation`.
>
> What do you think?
>
> Jiang Yan Xu wrote:
> Thanks for the reply. I've listed a few critia that I think could explain
> their subtle differences but I try to see if making such a distinction leads
> to simplity and clarity of the code. i.e., would everyone agree with the way
> we make such distinction and would it help readers understand the code
> better. If it's controversial, then perhaps we should aim for consistently. :)
>
> In terms of code in this file, yes I agree there's inconsistency and we
> should address it. To that end I think:
>
> To 1): The question to me is to choose between *OfferedResources ->
> Allocation* or *Allocation -> OfferedResources*. I am leaning towards
> *OfferedResources -> Allocation* and FWIW `Allocation` is a more established
> usage than `OfferedResources` in this file.
>
> To 2) and 3): sorry I failed to see the fundamental difference between
> the benchmarks and the unit tests here in terms of `offers` vs `allocations`.
> The unit tests, espcially later-added ones, follow basically the same pattern
> as the benchmarks. The benchmarks are just different in the use of
> expectations vs. measurements and the number of iterations. It's not
> convincing to me that in some cases we need to call them OfferedResources and
> in others we should call them Allocations.
>
> So overall I think it's better to change to use the Allocation struct
> defined at the top to achieve consistency. It looks to me that all
> occurrences of `offers` in the log messages can be changed to `allocations`
> as well.
>
> Let me know what you think.
Hi Jiang Yan, are you mentioning the `Allocation` definition here
https://github.com/apache/mesos/blob/9c6097f063405279efc07eec22457c2059653f07/src/tests/hierarchical_allocator_tests.cpp#L79-L83
? They are a bit different:
```
struct Allocation
{
FrameworkID frameworkId;
hashmap resources;
};
```
```
struct Allocation
{
FrameworkID frameworkId;
SlaveID slaveId;
Resources resources;
};
```
The above two is a bit difference: The first `Allocation` is an allocation for
a framework and the second `Allocation` is actullay an `offer` as one framework
can have multiple such `Allocation`.
So it may not accurate to use `allocations` in the benchmark test as they are
actually `offers` for each framework, comments? Thanks.
- Guangya
---
This is an automatically generated e-mail. To reply, visit:
Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
> On Aug. 25, 2016, 3:56 p.m., Jiang Yan Xu wrote: > > I don't know. Making such distinction feels like splitting hairs to me. > > Yeah there are subtle differences between the two concepts but to me: > > > > - `offerCallback` may not be the best name what we could have come up with > > even though in `Master::offer()` we do construct offers. > > - When offer objects are constructed, we call the resources in them > > "offered resources". > > - When we parse resources from offer objects, we call them "offered > > resources". > > - In this test, namely, the "Allocator" tests, we are indeed only > > allocating resources. (I don't see "Offer" objects being created). > > > > Later if the allocator actually hands out offer objects, then we probably > > need to change the tests to use offers directly but right now I think 1) > > "allocation" is most correct, 2) it's not a big deal if people treat them > > as synonymous terms. > > > > That said, I think reusing the "Allocation" defined at the top of the test > > makes sense. > > Guangya Liu wrote: > Thanks Jiang Yan, there are acutally some discussion here > https://reviews.apache.org/r/50677/ , the reason that I want to update this > is because: > > 1) Make the code consistent, in the allocator test cases, some benchmark > test using `OfferedResources` while some using `Allocation`, when people want > to add new benchmark test, they will be confused: Which one shall I use, > `OfferedResources` or `Allocation`, so here we need to make the code > consistent with each other for better reading, easy to maintain and > consistent. > > 2) Seems here using `OfferedResources` maybe better, as here we are > actually constructing the `offers` on each agent; But the `Allocation` is an > allocator concept and it means the resources for a specified framework. Also > all of the log messages are highlighting `offer` in benchmark test such as > here > https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3885 > > 3) The benchmark test is focusing on `offers` even though the allocator > know nothing about `offer`, but here we are just simulating `offer > operations` in benchmark test. For other unit test cases other than benchmark > test, they are focusing on `allocations` for each framework, so it is using > `Allocation`. > > What do you think? Thanks for the reply. I've listed a few critia that I think could explain their subtle differences but I try to see if making such a distinction leads to simplity and clarity of the code. i.e., would everyone agree with the way we make such distinction and would it help readers understand the code better. If it's controversial, then perhaps we should aim for consistently. :) In terms of code in this file, yes I agree there's inconsistency and we should address it. To that end I think: To 1): The question to me is to choose between *OfferedResources -> Allocation* or *Allocation -> OfferedResources*. I am leaning towards *OfferedResources -> Allocation* and FWIW `Allocation` is a more established usage than `OfferedResources` in this file. To 2) and 3): sorry I failed to see the fundamental difference between the benchmarks and the unit tests here in terms of `offers` vs `allocations`. The unit tests, espcially later-added ones, follow basically the same pattern as the benchmarks. The benchmarks are just different in the use of expectations vs. measurements and the number of iterations. It's not convincing to me that in some cases we need to call them OfferedResources and in others we should call them Allocations. So overall I think it's better to change to use the Allocation struct defined at the top to achieve consistency. It looks to me that all occurrences of `offers` in the log messages can be changed to `allocations` as well. Let me know what you think. - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50868/#review146878 --- On Aug. 5, 2016, 2:50 p.m., Guangya Liu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50868/ > --- > > (Updated Aug. 5, 2016, 2:50 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > --- > > The current benchmark test of `SuppressOffers` is using `Allocation` > to specify the resources from an `OfferCallback`, but here seems > using `OfferedResources` maybe better, as here we are actually > constructing the offers on each agent, but the `Allocation` is an > allocator concept and it means the resources allocated for a specified > framework. > > > Diffs > - > >
Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
> On 八月 25, 2016, 10:56 p.m., Jiang Yan Xu wrote: > > I don't know. Making such distinction feels like splitting hairs to me. > > Yeah there are subtle differences between the two concepts but to me: > > > > - `offerCallback` may not be the best name what we could have come up with > > even though in `Master::offer()` we do construct offers. > > - When offer objects are constructed, we call the resources in them > > "offered resources". > > - When we parse resources from offer objects, we call them "offered > > resources". > > - In this test, namely, the "Allocator" tests, we are indeed only > > allocating resources. (I don't see "Offer" objects being created). > > > > Later if the allocator actually hands out offer objects, then we probably > > need to change the tests to use offers directly but right now I think 1) > > "allocation" is most correct, 2) it's not a big deal if people treat them > > as synonymous terms. > > > > That said, I think reusing the "Allocation" defined at the top of the test > > makes sense. Thanks Jiang Yan, there are acutally some discussion here https://reviews.apache.org/r/50677/ , the reason that I want to update this is because: 1) Make the code consistent, in the allocator test cases, some benchmark test using `OfferedResources` while some using `Allocation`, when people want to add new benchmark test, they will be confused: Which one shall I use, `OfferedResources` or `Allocation`, so here we need to make the code consistent with each other for better reading, easy to maintain and consistent. 2) Seems here using `OfferedResources` maybe better, as here we are actually constructing the `offers` on each agent; But the `Allocation` is an allocator concept and it means the resources for a specified framework. Also all of the log messages are highlighting `offer` in benchmark test such as here https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3885 3) The benchmark test is focusing on `offers` even though the allocator know nothing about `offer`, but here we are just simulating `offer operations` in benchmark test. For other unit test cases other than benchmark test, they are focusing on `allocations` for each framework, so it is using `Allocation`. What do you think? - Guangya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50868/#review146878 --- On 八月 5, 2016, 9:50 p.m., Guangya Liu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50868/ > --- > > (Updated 八月 5, 2016, 9:50 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > --- > > The current benchmark test of `SuppressOffers` is using `Allocation` > to specify the resources from an `OfferCallback`, but here seems > using `OfferedResources` maybe better, as here we are actually > constructing the offers on each agent, but the `Allocation` is an > allocator concept and it means the resources allocated for a specified > framework. > > > Diffs > - > > src/tests/hierarchical_allocator_tests.cpp > cbed333f497016fe2811f755028796012b41db77 > > Diff: https://reviews.apache.org/r/50868/diff/ > > > Testing > --- > > make > make check > > ``` > ./bin/mesos-tests.sh --benchmark > --gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0" > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 > Using 1000 agents and 1 frameworks > Added 1 frameworks in 841us > Added 1000 agents in 480991us > allocate() took 264196us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 270110us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 270098us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 264314us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 268732us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 > (4511 ms) > [--] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test (4511 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (4522 ms total) > [ PASSED ] 1 test. > ``` > > > Thanks, > > Guangya Liu > >
Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50868/#review146878 --- I don't know. Making such distinction feels like splitting hairs to me. Yeah there are subtle differences between the two concepts but to me: - `offerCallback` may not be the best name what we could have come up with even though in `Master::offer()` we do construct offers. - When offer objects are constructed, we call the resources in them "offered resources". - When we parse resources from offer objects, we call them "offered resources". - In this test, namely, the "Allocator" tests, we are indeed only allocating resources. (I don't see "Offer" objects being created). Later if the allocator actually hands out offer objects, then we probably need to change the tests to use offers directly but right now I think 1) "allocation" is most correct, 2) it's not a big deal if people treat them as synonymous terms. That said, I think reusing the "Allocation" defined at the top of the test makes sense. - Jiang Yan Xu On Aug. 5, 2016, 2:50 p.m., Guangya Liu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50868/ > --- > > (Updated Aug. 5, 2016, 2:50 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > --- > > The current benchmark test of `SuppressOffers` is using `Allocation` > to specify the resources from an `OfferCallback`, but here seems > using `OfferedResources` maybe better, as here we are actually > constructing the offers on each agent, but the `Allocation` is an > allocator concept and it means the resources allocated for a specified > framework. > > > Diffs > - > > src/tests/hierarchical_allocator_tests.cpp > cbed333f497016fe2811f755028796012b41db77 > > Diff: https://reviews.apache.org/r/50868/diff/ > > > Testing > --- > > make > make check > > ``` > ./bin/mesos-tests.sh --benchmark > --gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0" > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 > Using 1000 agents and 1 frameworks > Added 1 frameworks in 841us > Added 1000 agents in 480991us > allocate() took 264196us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 270110us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 270098us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 264314us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 268732us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 > (4511 ms) > [--] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test (4511 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (4522 ms total) > [ PASSED ] 1 test. > ``` > > > Thanks, > > Guangya Liu > >
Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50868/#review145025 --- Patch looks great! Reviews applied: [50693, 50695, 50866, 50868] 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. 5, 2016, 9:50 p.m., Guangya Liu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50868/ > --- > > (Updated Aug. 5, 2016, 9:50 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > --- > > The current benchmark test of `SuppressOffers` is using `Allocation` > to specify the resources from an `OfferCallback`, but here seems > using `OfferedResources` maybe better, as here we are actually > constructing the offers on each agent, but the `Allocation` is an > allocator concept and it means the resources allocated for a specified > framework. > > > Diffs > - > > src/tests/hierarchical_allocator_tests.cpp > cbed333f497016fe2811f755028796012b41db77 > > Diff: https://reviews.apache.org/r/50868/diff/ > > > Testing > --- > > make > make check > > ``` > ./bin/mesos-tests.sh --benchmark > --gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0" > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 > Using 1000 agents and 1 frameworks > Added 1 frameworks in 841us > Added 1000 agents in 480991us > allocate() took 264196us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 270110us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 270098us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 264314us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > allocate() took 268732us to make 1000 offers with 0 out of 1 frameworks > suppressing offers > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 > (4511 ms) > [--] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test (4511 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (4522 ms total) > [ PASSED ] 1 test. > ``` > > > Thanks, > > Guangya Liu > >
Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50868/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- The current benchmark test of `SuppressOffers` is using `Allocation` to specify the resources from an `OfferCallback`, but here seems using `OfferedResources` maybe better, as here we are actually constructing the offers on each agent, but the `Allocation` is an allocator concept and it means the resources allocated for a specified framework. Diffs - src/tests/hierarchical_allocator_tests.cpp cbed333f497016fe2811f755028796012b41db77 Diff: https://reviews.apache.org/r/50868/diff/ Testing --- make make check ``` ./bin/mesos-tests.sh --benchmark --gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0" [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test [ RUN ] SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 Using 1000 agents and 1 frameworks Added 1 frameworks in 841us Added 1000 agents in 480991us allocate() took 264196us to make 1000 offers with 0 out of 1 frameworks suppressing offers allocate() took 270110us to make 1000 offers with 0 out of 1 frameworks suppressing offers allocate() took 270098us to make 1000 offers with 0 out of 1 frameworks suppressing offers allocate() took 264314us to make 1000 offers with 0 out of 1 frameworks suppressing offers allocate() took 268732us to make 1000 offers with 0 out of 1 frameworks suppressing offers [ OK ] SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/0 (4511 ms) [--] 1 test from SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test (4511 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (4522 ms total) [ PASSED ] 1 test. ``` Thanks, Guangya Liu
