mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 41658: Track the allocation candidates to bound the allocation queue.
Date Tue, 21 Jun 2016 17:14:34 GMT


> On June 21, 2016, 8:25 a.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 368
> > <https://reviews.apache.org/r/41658/diff/1/?file=1174679#file1174679line368>
> >
> >     Instead of using a counter I think the following is equivalent but simpler:
> >     
> >     ```
> >     bool allocationPending;
> >     ```
> >     
> >     So instead of always dispatching into the queue which results in many noops,
we don't dispatch if we know there is already an allocation pending in the queue. We merely
update `allocationCandidates`.
> >     
> >     Later after the allocation is run we reset the bool.
> >     
> >     Make sense?
> 
> James Peach wrote:
>     That's not the same thing. The allocation counter lets you take the the *last* allocation
in the queue, rather than any. For example, imagine the dispatch queue contains TTTTATTT (T
is some event, A is an allocation). The allocation counter turns this into TTTTATTTA but only
the last A would be executed. So if T adds allocation candidates you would end up doing fewer
allocations.
>     
>     Note that I'm not claiming that this makes a difference in practice; it could well
be that the boolean flag is good enough for the common cases you are trying to address. Just
giving the background for the counter.

Got it. The exact sequence my vary but I think the end result is the same "prevent unnecessary
allocation calls".


Just to clarify your example:

The A in TTTTATTT you mentioned is the batch() call that gets inserted into the queue right?
If I rename it as B to differentiate it from the allocate() call, then:

- With the counter based algorithm: TTTTBTTT -> TTTTBTTTAAAA**A**AA**A** rigth? (The bold
ones are the real allocations, the first due to `force == true`)
- With the boolean based algorithm: TTTTBTTT -> TTTTBTTT**A**. To prevent starvation of
the batch allocation we can do an allocation synchronously in batch(), so this becomes TTTTBTTT
-> TTTT**A**TTT**A**.

Does it look right?


> On June 21, 2016, 8:25 a.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1080
> > <https://reviews.apache.org/r/41658/diff/1/?file=1174680#file1174680line1080>
> >
> >     This uses `true` but I don't see why batch needs to be different.
> 
> James Peach wrote:
>     The force is to ensure that allocations are not starved out. If there is a lot of
churn and we keep delaying the actial allocation, you can imagine a scenario where the allocation
is deferred indefinitely. This ensures that allocations occur at least at the batch interval.

I see. With the boolean-based approach it will not be deferred indefinitely but to ensure
we do an allocation at least per each batch interval we can do allocate() synchronously in
batch (in my example above). That'll be a stronger guarantee than dispatching again but with
`force == true` right?


> On June 21, 2016, 8:25 a.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1137
> > <https://reviews.apache.org/r/41658/diff/1/?file=1174680#file1174680line1137>
> >
> >     No need to rename the method?
> 
> James Peach wrote:
>     It made it clearer since there are a number of overloads of ``allocate()``.

I see. But I think the current situation is not too bad and if we rename it we probably need
to do others so it's consistent.

We currently have:

```
  // Allocate any allocatable resources.
  void allocate();

  // Allocate resources just from the specified slave.
  void allocate(const SlaveID& slaveId);

  // Allocate resources from the specified slaves.
  void allocate(const hashset<SlaveID>& slaveIds);
```

If we rename just one we are stilling leave the other two overloaded... but I guess I can't
see the confusion, they all do the same thing, just with different input, sounds like what
overloads are for.


- Jiang Yan


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


On Dec. 22, 2015, 11:56 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41658/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2015, 11:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When there is churn in the cluster, frequent resource allocation
> is required.  Maintain a set of allocation candidates so that we
> don't end up running the same allocation multiple times.
> 
> This review is just for feedback. Not proposing it to be berged at this time.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 86ea5a402ed67f8f22f11d5730147cd907d66a08

>   src/master/allocator/mesos/hierarchical.cpp 775182515dcb52bd873ecdf98c827320251a59c8

> 
> Diff: https://reviews.apache.org/r/41658/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> James Peach
> 
>


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