----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40632/#review115263 -----------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp (lines 1918 - 1919) <https://reviews.apache.org/r/40632/#comment176206> Note: This separation comment is probably unnecessary. As long as the new tests are named properly (i.e. AllocationSlack...) we can tell them apart quite easily. src/tests/hierarchical_allocator_tests.cpp (line 1922) <https://reviews.apache.org/r/40632/#comment176207> s/resources on a slave that are statically reserved/statically reserved resources/ src/tests/hierarchical_allocator_tests.cpp (lines 1924 - 1925) <https://reviews.apache.org/r/40632/#comment176211> s/to any role and/to any role, and/ s/for a role are only offered to frameworks in that role/are still only allocated to the reserved role/ src/tests/hierarchical_allocator_tests.cpp (lines 1952 - 1960) <https://reviews.apache.org/r/40632/#comment176212> It would be great to have a comment above this :) src/tests/hierarchical_allocator_tests.cpp (lines 1995 - 1997) <https://reviews.apache.org/r/40632/#comment176198> Consider moving this test to after you've implemented optimistic offers for `Allocator::recoverResources`. src/tests/hierarchical_allocator_tests.cpp (lines 2045 - 2047) <https://reviews.apache.org/r/40632/#comment176199> Consider moving this test to after you've implemented optimistic offers for `Allocator::recoverResources`. src/tests/hierarchical_allocator_tests.cpp (line 2089) <https://reviews.apache.org/r/40632/#comment176213> s/was not count in Quota/can be allocated regardless of Quota/ src/tests/hierarchical_allocator_tests.cpp (line 2107) <https://reviews.apache.org/r/40632/#comment176215> It may improve readability to split up that big comment. Suggestion: Set quota for the quota'd role. In the first allocation cycle, this effectively prevents `agent1` from being offered to any role except the quota'd role (as doing so would put the quota'd role under quota). src/tests/hierarchical_allocator_tests.cpp (line 2111) <https://reviews.apache.org/r/40632/#comment176218> Continued suggestion: Add a framework that can only be offered non-revocable resources from `agent2`. src/tests/hierarchical_allocator_tests.cpp (lines 2116 - 2123) <https://reviews.apache.org/r/40632/#comment176221> Continued suggestion: Check that `framework1` gets offered resources from `agent2` as well as allocation slack from `agent1`. - Joseph Wu On Jan. 19, 2016, 12:03 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40632/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2016, 12:03 a.m.) > > > Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van > Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu. > > > Bugs: MESOS-4145 > https://issues.apache.org/jira/browse/MESOS-4145 > > > Repository: mesos > > > Description > ------- > > Enabled oversubscribed resources for reservations in allocator. > The allocator part including 5 patches: > 1) https://reviews.apache.org/r/40632 Enabled oversubscribed resources for > reservations in allocator > 2) https://reviews.apache.org/r/41847 Updated allocation slack when slave was > updated. > 3) https://reviews.apache.org/r/41791 Updated allocation slack for dynamic > reserve (1/3). > 4) https://reviews.apache.org/r/42113 Handle unreserve logic for dynamic > reservation (2/3). > 5) https://reviews.apache.org/r/42194 Handle unreserve logic for dynamic > reservation (3/3). > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 48acde69b1a2f305b568a7e322a58708063dd30a > src/tests/hierarchical_allocator_tests.cpp > 9362dd306497ba01e0f387c3862456cdcac6f863 > > Diff: https://reviews.apache.org/r/40632/diff/ > > > Testing > ------- > > make > make check > GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" > --verbose > > > Thanks, > > Guangya Liu > >
