Re: Review Request: Made DRF per user and log(N)

[Thomas Marshall <tw...@gmail.com>]

> On July 22, 2012, 6:21 a.m., Benjamin Hindman wrote:
> > src/local/local.hpp, line 26
> > <https://reviews.apache.org/r/5599/diff/9/?file=124305#file124305line26>
> >
> >     I would just keep the file called allocator.hpp, especially if we want to wrap the AllocatorProcess in an Allocator.

Fixed.


> On July 22, 2012, 6:21 a.m., Benjamin Hindman wrote:
> > src/master/sorter.hpp, line 117
> > <https://reviews.apache.org/r/5599/diff/9/?file=124314#file124314line117>
> >
> >     Let's call this 'sort' (the main functionality of the class).

Fixed.


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5599/#review9261
-----------------------------------------------------------


On July 18, 2012, 6:19 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5599/
> -----------------------------------------------------------
> 
> (Updated July 18, 2012, 6:19 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 10f1101 
>   src/local/local.hpp 8ae5b9e 
>   src/local/local.cpp 46537d7 
>   src/master/allocator.hpp e55b9ec 
>   src/master/allocator_process.hpp PRE-CREATION 
>   src/master/main.cpp 8d38fb1 
>   src/master/master.hpp ad2ec25 
>   src/master/master.cpp b5e09d8 
>   src/master/simple_allocator_process.hpp PRE-CREATION 
>   src/master/simple_allocator_process.cpp PRE-CREATION 
>   src/master/sorter.hpp PRE-CREATION 
>   src/master/sorter.cpp PRE-CREATION 
>   src/tests/allocator_tests.cpp b3db13d 
>   src/tests/fault_tolerance_tests.cpp f892282 
>   src/tests/master_detector_tests.cpp 758c8b9 
>   src/tests/master_tests.cpp b586984 
>   src/tests/resource_offers_tests.cpp c004772 
>   src/tests/slave_tests.cpp e9b25ba 
>   src/tests/utils.hpp ced8592 
> 
> Diff: https://reviews.apache.org/r/5599/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>