----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5599/#review10365 -----------------------------------------------------------
Ship it! Ye haw! src/master/allocator.cpp <https://reviews.apache.org/r/5599/#comment22001> Let's call this UserFrameworkAllocatorProcess or something more descriptive than SimpleAllocatorProcess. src/master/drf_sorter.hpp <https://reviews.apache.org/r/5599/#comment22004> s|sorter|master/sorter| src/master/drf_sorter.hpp <https://reviews.apache.org/r/5599/#comment22003> Let's not use the typedef. src/master/drf_sorter.hpp <https://reviews.apache.org/r/5599/#comment22007> Kill 'virtual'. src/master/drf_sorter.hpp <https://reviews.apache.org/r/5599/#comment22008> Ditto. src/master/drf_sorter.hpp <https://reviews.apache.org/r/5599/#comment22009> Ditto. src/master/drf_sorter.hpp <https://reviews.apache.org/r/5599/#comment22012> s/clients/client names/ src/master/drf_sorter.cpp <https://reviews.apache.org/r/5599/#comment22006> master/ src/master/drf_sorter.cpp <https://reviews.apache.org/r/5599/#comment22011> Always remove from allocations. src/master/drf_sorter.cpp <https://reviews.apache.org/r/5599/#comment22025> s/client/name/ src/master/flags.hpp <https://reviews.apache.org/r/5599/#comment22021> s/allocator/sorter src/master/flags.hpp <https://reviews.apache.org/r/5599/#comment22022> Ditto. src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22026> const& src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22013> We should swap the semantics of these please. src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22020> Use new self. src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22014> Swap these too. src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22023> users ;) src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22017> const& src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22018> const& src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22015> Kill this allocate. src/master/simple_allocator_process.hpp <https://reviews.apache.org/r/5599/#comment22016> Format this line (and others that wrap beyond 80 characters). src/master/sorter.hpp <https://reviews.apache.org/r/5599/#comment22024> s|/should|/ should| - Benjamin Hindman On Aug. 13, 2012, 11:59 p.m., Thomas Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5599/ > ----------------------------------------------------------- > > (Updated Aug. 13, 2012, 11:59 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Description > ------- > > Rewrote the DRF algorithm in DominantShareAllocator to calculate shares on a > per user, rather than per framework, basis and to store those shares in > sorted order so that allocations have log(n) time complexity instead of n^2. > > Significant restructuring of the allocator code to make writing new > allocators easier: > > - The Allocator class is renamed AllocatorProcess. > - A new Allocator class is introduced which knows only about "elements" > (either user names or frameworkIds) and how many resources they have been > allocated, and then has an interface for retrieving the elements in the order > they should be allocated to. > - SimpleAllocatorProcess which can be used with different Allocators to carry > out different allocation policies, including the possibility of having > different policies for the per-user and per-framework allocations. > > Why is this restructuring a good thing? > Now that we're doing allocations based on users, rather than frameworks, we > have to do two levels of allocation - determine which user to give resources > to, then determine which of their frameworks to actually make the offer to. > Rather than hard-coding the allocation policies in, the new Allocator class > allows us to reuse the same code to do both levels of allocation, and it > allows us to easily swap different policies in or mix and match. You can even > imagine some day having an allocator that allows each user to pick the > per-framework allocation policy they prefer. > > Merely giving SimpleAllocatorProcess different Allocators is not sufficient > to implement all allocation policies - only those where all resources are > considered equal and there is no revocation. But, you can always create new > AllocatorProcess subclasses (there will soon be a code review where I > implement a static allocator this way) and these new AllocatorProcess classes > can still reuse Allocators as appropriate. > > It also makes testing the allocator logic a lot easier since we can > instantiate Allocators, give them arbitrary elements, and check that they > return the right thing, instead of worrying about instantiating > slaves/frameworks and using mock objects. > > > This addresses bugs MESOS-225 and MESOS-226. > https://issues.apache.org/jira/browse/MESOS-225 > https://issues.apache.org/jira/browse/MESOS-226 > > > Diffs > ----- > > src/Makefile.am b0cb6cc > src/local/local.hpp 8ae5b9e > src/local/local.cpp 42dbf06 > src/master/allocator.hpp 8805066 > src/master/allocator.cpp PRE-CREATION > src/master/drf_sorter.hpp PRE-CREATION > src/master/drf_sorter.cpp PRE-CREATION > src/master/flags.hpp f66f0a3 > src/master/main.cpp 172772f > src/master/master.hpp d21ff3c > src/master/master.cpp 0eca2fa > src/master/simple_allocator_process.hpp PRE-CREATION > src/master/sorter.hpp PRE-CREATION > src/tests/allocator_tests.cpp 98efb0d > src/tests/allocator_zookeeper_tests.cpp PRE-CREATION > src/tests/fault_tolerance_tests.cpp f892282 > src/tests/gc_tests.cpp 2f0bdde > src/tests/master_detector_tests.cpp 758c8b9 > src/tests/master_tests.cpp b586984 > src/tests/resource_offers_tests.cpp 1366032 > src/tests/utils.hpp caf5926 > > Diff: https://reviews.apache.org/r/5599/diff/ > > > Testing > ------- > > make check on Lion > > > Thanks, > > Thomas Marshall > >
