mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 57788: Changed allocator to skip batch allocate on weight change.
Date Tue, 21 Mar 2017 21:41:29 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?

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


- Benjamin


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


On March 20, 2017, 6:14 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57788/
> -----------------------------------------------------------
> 
> (Updated March 20, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Benjamin Mahler, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When the weight is changed for one or more roles, the allocator will no
> longer do a batch allocation. The weight change will be reflected in the
> next allocation, which should be sufficient.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 8d54a8cca1bb478f4437f68c5e14f66a9f9bb9e9

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


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