mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rukletsov" <ruklet...@gmail.com>
Subject Re: Review Request 35947: Added a new API call 'updateAvailable' to the allocator.
Date Thu, 09 Jul 2015 16:20:13 GMT


> On July 8, 2015, 5:59 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, lines 133-135
> > <https://reviews.apache.org/r/35947/diff/1/?file=993649#file993649line133>
> >
> >     And we introduce a libprocess dependency into `Allocator` interface. I think
it's a prominent step, right now an allocator implementation is not even required to be asynchronous.
I don't want to say it's a bad change, I want us to think a bit more about the consequences
of such step. Maybe it is worth discussing on the dev list.
> 
> Jie Yu wrote:
>     Most of our module interfaces depends on libprocess (e.g., resource estimator, qos
controller, isolator, etc.). Why do you think this is a prominent step?
>     
>     > right now an allocator implementation is not even required to be asynchronous
>     
>     I think at the time we introduced modules, we agreed on that internal interfaces
are subject to change without needing a formal deprecation cycle. It's up to the module developer
to use proper versioning to deal with changes in the interfaces.

To clarify, I think it's fine updating an interface, but giving the growing complexity of
allocator, introducing a libprocess dependency doesn't make it easier : ). However, if we
all share same concerns and are eager to refactor the allocator interface in the near future,
I'm fine with the change.


- Alexander


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


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message