> On Jan. 15, 2016, 6:11 p.m., Joseph Wu wrote: > > src/master/allocator/mesos/hierarchical.cpp, lines 1291-1319 > > <https://reviews.apache.org/r/40632/diff/15/?file=1195627#file1195627line1291> > > > > None of this should be necessary: > > > > 1) You should have all the allocation slack inside `available`. > > 2) We don't have allocation slack ACLs in the MVP, so all allocation > > slack should fall under `available.unreserved`. If we do have ACLs, some > > of the allocation slack will be accounted for in `available.reserved(role)`. > > 3) You shouldn't need to recalculate this during `::allocate`. > > > > If any of the above are not true, you probably have a problem in the > > `Resource` helpers or in `::addSlave`. > > Guangya Liu wrote: > We can take a look at the following cases: > 1) total resources cpus(r1):100 > 2) register a framework f1, cycle 1: allocated > cpus(r1):100;cpu(*){ALLOCATION_SLACK}:100 to f1 > 3) f1 recovered cpu(*){ALLOCATION_SLACK}:100 back > 4) register another framwork f2, f2 get nothing here as f1 used up all > reserved resoures and there is no allocation slack now. > > The above logic make sure that step 4) can always get the latest > remaining allocation slack. > > Joseph Wu wrote: > The step 3) in your case is presumably because you haven't implemented > all the allocator methods yet (as of this review in the chain), like > `recoverResources`, `updateAllocation`, and `updateAvailable`. > > If so, you should consider moving the test additions/changes before/after > (this lets us know what your intended behavior is) the allocator changes. > > Guangya Liu wrote: > I was updating the comments here to make it more clear. > > // Calculate the `remainingAllocationSlack` if the framework can > // use revocable resources and reservation oversubscription also > // enabled. The `remainingAllocationSlack` need to exclude the > // stateless reserved resources allocated in previous allocation > // cycle.
I would still **highly recommend** that you don't recalculate the allocation slack inside a doubly-nested for-loop. And I'll reinterate: if you can't find the allocation slack inside the `available` Resources object, then you're doing something wrong. You should probably double-check if this is the case. - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40632/#review114831 ----------------------------------------------------------- On Jan. 19, 2016, 10:25 p.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40632/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2016, 10:25 p.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 > e32ee4aa3ed9793bb5a99233e699e5cc2bdd796b > src/tests/hierarchical_allocator_tests.cpp > 953712149bd951789beb29c72779c4ac65aa48dc > > Diff: https://reviews.apache.org/r/40632/diff/ > > > Testing > ------- > > make > make check > GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" > --verbose --gtest_repeat=100 --gtest_shuffle > > > Thanks, > > Guangya Liu > >
