> On Aug. 23, 2016, 2:26 a.m., Alexander Rukletsov wrote: > > src/master/allocator/mesos/hierarchical.cpp, lines 276-279 > > <https://reviews.apache.org/r/51027/diff/2/?file=1481820#file1481820line276> > > > > Probably extract this snippet in a function, e.g. > > `conditionalAllocate()`? > > Jiang Yan Xu wrote: > I thought about it but was struggling to find a short and clear method > name. So to describe the function in a full sentense it's "dispatch an > allocate() call if the condition `!allocationPending` is met". I think > `conditionalAllocate()` is OK but not great, it's not clear what the > condition is and not clear about the dispatch. I agree it's worth doing if we > can abstract this out without needing to explain what each line of a 4-line > method is doing in the comment... > > Alexander Rukletsov wrote: > Let's focus on _why_ instead of _how_. You want to ensure that one and > only one event allocation happens after the event. Dispatch+flag is _how_ you > achieve this, which may change in future. Here are some suggestions: > - `ensureAllocate` > - `ensureAllocation` > - `allocateOnce` > > Side note: "unique dispatch", i.e. a dispatch that succeeds only if there > are no identical messages in the actor's mailbox, looks like something we may > need in other places. Maybe we should consider adding such functionality into > libprocess.
Good point. I like `ensureAllocation()` better than the rest. ``` // Ensure an allocation run is pending in the event queue (i.e,. one is dispatched otherwise). // TODO: Consider generalizing this into a "unique dispatch" functionality in libprocess. void ensureAllocation(); ``` Yeah I like the idea of "unique dispatch", see the TODO. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51027/#review146499 ----------------------------------------------------------- On Aug. 23, 2016, 1:49 a.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51027/ > ----------------------------------------------------------- > > (Updated Aug. 23, 2016, 1:49 a.m.) > > > Review request for mesos, Benjamin Mahler, Guangya Liu, James Peach, Klaus > Ma, and Jiang Yan Xu. > > > Bugs: MESOS-3157 > https://issues.apache.org/jira/browse/MESOS-3157 > > > Repository: mesos > > > Description > ------- > > - Triggered allocations dispatch allocate() only > if there is no pending allocation in the queue. > - Allocation candidates are accumulated and only > cleared when enqueued allocations are processed. > - Batched allocations are handled synchronously. > > - Carrying over work from https://reviews.apache.org/r/41658/ and added the > previous reviewers > - Specifically, this patch introduces the boolean flag pendingAllocation, > which when set on event > triggered allocations, will prevent additional no-op allocations: the flag > is cleared when > the enqueued allocation is processed, subsequent event triggered > allocations will update a set > of allocation candidates rather than dispatching an additional allocate(). > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 > src/master/allocator/mesos/hierarchical.cpp > 234ef98529964a0b6d3f132426a6c8ccbb1263ee > > Diff: https://reviews.apache.org/r/51027/diff/ > > > Testing > ------- > > make check > > note: check without filters depends on https://reviews.apache.org/r/51028 > > With new benchmark https://reviews.apache.org/r/49617: > Sample output without 51027: > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22 > Using 10000 agents and 3000 frameworks > Added 3000 frameworks in 57251us > Added 10000 agents in 3.21345353333333mins > allocator settled after 1.61236038333333mins > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22 > (290578 ms) > > Sample output with 51027: > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22 > Using 10000 agents and 3000 frameworks > Added 3000 frameworks in 39817us > Added 10000 agents in 3.22860541666667mins > allocator settled after 25.525654secs > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22 > (220137 ms) > > > Thanks, > > Jacob Janco > >
