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