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