> On June 21, 2016, 3:25 p.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. > > Jiang Yan Xu wrote: > 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?
``force == true`` is for supporting the batch, so **A** and B are the same thing. Say your starting queue is TTTATT. Then some cluster event triggers another allocation. With the boolean flag approach you would know there is an allocation pending so you wouldn't dispatch a new one; you would end up with TTT**A**TT. With the counter approach, you would dispatch, so you would do TTTATT**A**, which could be a win if we assume that this let us accumulate more allocation candidates. Keep in mind that cluster changes are getting dispatched onto the allocator queue, so I don't think you can have a sequence of AAAA - you always have an intermediate T that would have to dispatch an A, so the worst case generated by the counter approach is TATATAT**A**. > On June 21, 2016, 3:25 p.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. > > Jiang Yan Xu wrote: > 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? Yes you are right. By dispatching ``batch``, the queue could grow unbounded if the allocation time is longer than the batch interval. Making this synchronous would be better. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41658/#review138796 ----------------------------------------------------------- On June 22, 2016, 4:32 a.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41658/ > ----------------------------------------------------------- > > (Updated June 22, 2016, 4:32 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 merged 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 > >
