mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil Conway <neil.con...@gmail.com>
Subject Re: Review Request 57788: Changed allocator to skip allocation on weight and quota changes.
Date Wed, 22 Mar 2017 18:43:00 GMT


> On March 20, 2017, 6:26 p.m., Benjamin Mahler wrote:
> > Hm.. seems like there was no test for this "optimization" if the tests pass, did
you do a scan of the allocator unit tests to be sure we're not leaving a stale test lying
around?
> 
> Neil Conway wrote:
>     The change was introduced here, which didn't include any tests: https://reviews.apache.org/r/41597/
> 
> Adam B wrote:
>     A test was added in a follow-up patch: https://reviews.apache.org/r/41672/diff but
that doesn't test the instant-rebalance behavior specifically.
>     I'm not sure I understand why you're removing this "optimization". I believe the
original intention was that if an operator updates weights, he/she would hope to see an immediate
impact (since changing `--weights` and restarting the master implies a rebalance). I suppose
a 1s delay is acceptable, and this does simplify the code. Was there any more specific motivation
here?
> 
> Benjamin Mahler wrote:
>     Yeah, I was thinking more about this last night and it's quite subtle.
>     
>     In the future, if we add the ability for us to "rebalance" the cluster by rescinding
offers to respect changes to the quota or weights, then clearly it seems necessary to induce
this immediately when the weights or quotas are changed.
>     
>     The current implementation of quotas and weights however, are such that when quota
or weights are modified it has no effect on existing allocations/offers. The only effect is
on the distribution of subsequent allocations of available resources. My thinking was therefore
that it seemed unfortunate to "flush" the queue of available resources that need allocation
because of a weight or quota change, since we mostly care about **re**-balancing and we cannot
do that currently.
>     
>     This definitely will not be obvious to people, so we should have comments in the
quota and weight setters along the lines of:
>     
>     ```
>     // Subsequent allocations will reflect the changes to the (weights|quotas).
>     //
>     // If we add the ability for (weight|quota) changes to incur a rebalancing
>     // of the offered resources, then we will need to trigger it immediately
>     // here. For now, since (weight|quota) changes only affect the subsequent
>     // allocations of available resources, we don't interrupt the allocation
>     // loop and simply allow the next run to pick up the changes.
>     ```

Added comments to this effect, and also changed the `setQuota` and `removeQuota` to skip triggering
an allocation.


- Neil


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


On March 22, 2017, 6:36 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57788/
> -----------------------------------------------------------
> 
> (Updated March 22, 2017, 6:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Bannier, Benjamin Mahler,
and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Changing weight or quota will no longer trigger a batch allocation.
> Since the allocator does not rebalance currently offered resources to
> reflect changes to weight or quota, doing a batch allocation is not
> useful; instead, the updated quota/weight values will be reflected on
> the next allocation.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 8d54a8cca1bb478f4437f68c5e14f66a9f9bb9e9

>   src/tests/hierarchical_allocator_tests.cpp e343dc37bd7136f0f6dd5dbc22a25cabe715038d

> 
> 
> Diff: https://reviews.apache.org/r/57788/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


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