> On June 9, 2017, 12:22 p.m., Benjamin Mahler wrote: > > src/tests/hierarchical_allocator_tests.cpp > > Lines 4606-4607 (patched) > > <https://reviews.apache.org/r/58510/diff/1/?file=1693724#file1693724line4606> > > > > I was a bit confused about why this test was using quota, then I > > figured out that this is how you wanted to ensure that the allocation goes > > to the descendant. > > > > We should document this at the top of the test, or here, to describe > > that we leverage quota to test this. > > > > Simpler to understand would be to not use quota and add two agents, > > expecting one agent to have been allocated to the ancestor and descendant, > > each. Does that not work? > > Jay Guo wrote: > Since we made changes for both quota and fair share stage of allocation, > I'm using quota to make sure we cover both logic paths. I updated the test > comments, please take a look.
I would suggest: ``` // This test ensures that resources reserved to ancestor roles can be offered // to their descendants. Since both quota and fair-share stages perform // reservation allocations, we use a quota'd role to test both cases. ``` - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/#review177509 ----------------------------------------------------------- On June 13, 2017, 8:40 a.m., Jay Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58510/ > ----------------------------------------------------------- > > (Updated June 13, 2017, 8:40 a.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-7149 > https://issues.apache.org/jira/browse/MESOS-7149 > > > Repository: mesos > > > Description > ------- > > Added a test for hierarchical reservation. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 631e800e6566847e015ea2638cc1c2fe673855be > > > Diff: https://reviews.apache.org/r/58510/diff/2/ > > > Testing > ------- > > make check > > comment out > https://github.com/apache/mesos/blob/master/src/common/roles.cpp#L71-L74 and > `./bin/mesos-tests.sh > --gtest_filter="HierarchicalAllocatorTest.DISABLED_OfferAncestorReservationsToDescendantChild" > --gtest_also_run_disabled_tests` > > > Thanks, > > Jay Guo > >
