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


A high-level concern I would like to share with you guys, though it isn't 
directly related to this particular patch. I have a feeling that we don't 
really care about keeping the Allocator interface neat, brief, and concise, but 
rather abuse it and add random methods to it in order to satisfy our current 
needs for the built-in allocator. With several production-grade allocators on 
the market we won't be able to do this anymore. Shall we be more cautious about 
that? Maybe it is a good time to schedule allocator refactoring?

- Alexander Rukletsov


On June 26, 2015, 10:55 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35947/
> -----------------------------------------------------------
> 
> (Updated June 26, 2015, 10:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Hindman, Ben Mahler, 
> and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Needed to implement the master HTTP endpoints: `/reserve`, `/unreserve`, 
> `/create` and `/destroy`.
> 
> This API is similar to `updateSlave` which is currently very specific to 
> oversubscription.
> I considered consolidating `updateAvailable` and `updateSlave` but it will 
> require making offers be generated within the allocator to enable this.
> 
> In specific, `updateAvailable` could fail if there aren't sufficient 
> available resources to update, whereas `updateSlave` avoids failing by 
> keeping the allocator in an over-allocated state. For `updateSlave`, leaving 
> the allocator in an over-allocated state is ok. This is because it does not 
> modify resources therefore `total - allocated` will work out to __empty__. 
> `updateAvailable` cannot leave the allocator in an over-allocated state, 
> because it modifies resources, and therefore `total - allocated` is not 
> guaranteed to yield __empty__.
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 22992c0c77058af4fcd28aa8e4a1191693a16f44 
>   src/master/allocator/mesos/allocator.hpp 
> 72470ec7f56f84a9a9815c09adb88def90ef672f 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3264d145d52b48852878abf7ab9be29ab98208cc 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3258840135290cd008ca09235d18b7f093dafd2e 
>   src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
> 
> Diff: https://reviews.apache.org/r/35947/diff/
> 
> 
> Testing
> -------
> 
> (1) Added `HierarchicalAllocatorTest.updateAvailableSuccess` and 
> `HierarchicalAllocatorTest.updateAvailableFail`
> (2) `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Reply via email to