----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49375/#review140617 -----------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp (line 543) <https://reviews.apache.org/r/49375/#comment206000> s/roleSorter/`roleSorter` src/master/allocator/mesos/hierarchical.cpp (lines 716 - 723) <https://reviews.apache.org/r/49375/#comment206002> Just nit: I think that here the `role sorters` should include both `roleSorter` and `quotaRoleSorter`, so what about grouping the `sorters` into one code block as following? // Now, update the total resources in the role sorters by removing // the previous resources at this slave and adding the new resources. roleSorter->remove(slaveId, oldTotal); roleSorter->add(slaveId, updatedTotal.get()); quotaRoleSorter->remove(slaveId, oldTotal.nonRevocable()); quotaRoleSorter->add(slaveId, updatedTotal.get().nonRevocable()); - Guangya Liu On 七月 1, 2016, 9:47 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49375/ > ----------------------------------------------------------- > > (Updated 七月 1, 2016, 9:47 a.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-5698 > https://issues.apache.org/jira/browse/MESOS-5698 > > > Repository: mesos > > > Description > ------- > > The DRFSorter previously kept the total resources at each slave, along > with the total quantity of resources in the cluster. The latter figure > is what most of the clients of the sorter care about. In the few places > where the previous coding was using the per-slave total resources, it is > relatively easy to adjust the code to remove this dependency. > > As part of this change, remove `total()` and `update(const SlaveID& > slaveId, const Resources& resources)` from the Sorter interface. The > former was unused; for the latter, code that used it can instead be > rewritten to adjust the total resources in the cluster by first removing > the previous resources at a slave and then adding the new resources. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 38381237fa6ceb3f21fd0d4b07d7c3787f0129df > src/master/allocator/sorter/drf/sorter.hpp > 0aa1a71da4501a3b469d07538a043b4c1d74d688 > src/master/allocator/sorter/drf/sorter.cpp > 967290d4d1100208900b4b724422c3218abc23cb > src/master/allocator/sorter/sorter.hpp > 5ce6bb82b0127257d97daf0cea6d1d0db405bf83 > src/tests/sorter_tests.cpp 6fc58829892dc0223140f1b47593a3e5853cace5 > > Diff: https://reviews.apache.org/r/49375/diff/ > > > Testing > ------- > > make check on OSX and recent Arch Linux. > > > Thanks, > > Neil Conway > >
