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 Tue, 28 Jul 2015 13:24:57 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.
> 
> Alexander Rukletsov wrote:
>     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.
> 
> Michael Park wrote:
>     Hey Alex, I've filed [MESOS-3147](https://issues.apache.org/jira/browse/MESOS-3147)
to keep track of the allocator refactor work and to get take the discussion out of these reviews.
Please augment the ticket with your thoughts and plans, Thanks!

That's great Michael, thank you very much!


- Alexander


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


On July 24, 2015, 9:26 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35947/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, 9:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Hindman, Ben Mahler, and Jie
Yu.
> 
> 
> Bugs: MESOS-3146
>     https://issues.apache.org/jira/browse/MESOS-3146
> 
> 
> 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 69134e1c2664ca24a1ecd80a662c841311104a6a 
> 
> 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